From eeaf210e7872a7d631cdbc2960dc5a8fd581f2a6 Mon Sep 17 00:00:00 2001 From: Maximilian Stubhan Date: Thu, 10 Sep 2026 16:05:40 +0200 Subject: [PATCH] Let the same choice open notes and drafts The picker in the bell now governs both lists, so note_subscriptions is renamed to colleague_subscriptions -- a name that only mentions notes would mislead the next reader. Reading and writing a draft now reach differently far. hire_drafts_owner (for all) is split into four policies: select lets in your own drafts and those of the people you added, while insert/update/delete stay with the owner. A draft is unfinished work with no lock and no history; two people writing into the same row would overwrite each other silently. That split forces a change in the actions: a policy does not reject a write, it lets it hit no rows. saveHireDraft and deleteHireDraft now read the row count instead of reporting success over a row that never changed. The card shows a foreign draft with its author and without Fortsetzen or Loeschen -- offering a button that reliably ends in a database error is a promise without cover. check-schema-types.mjs learns `alter table ... rename to`; without it the drift check reports one rename as two errors. Co-Authored-By: Claude Opus 5 --- actions/hireDrafts.ts | 27 +++- actions/notes.ts | 13 +- app/(app)/page.tsx | 2 +- components/dashboard/DraftsCard.tsx | 54 ++++--- components/shell/NotesBell.tsx | 7 +- ...120000_abos_gelten_auch_fuer_entwuerfe.sql | 118 +++++++++++++++ docs/datenkatalog.md | 34 ++++- lib/dashboard-data.ts | 24 ++-- lib/entwuerfe.ts | 84 +++++++++++ lib/kollegen.ts | 28 ++++ lib/notes.ts | 10 +- lib/shell-data.ts | 12 +- lib/types.ts | 6 +- scripts/check-schema-types.mjs | 13 ++ tests/components/DraftsCard.test.tsx | 134 +++++++++++++++++ tests/unit/entwuerfe.test.ts | 136 ++++++++++++++++++ tests/unit/notes-visibility.test.ts | 2 +- 17 files changed, 650 insertions(+), 54 deletions(-) create mode 100644 db/migrations/20260910120000_abos_gelten_auch_fuer_entwuerfe.sql create mode 100644 lib/entwuerfe.ts create mode 100644 lib/kollegen.ts create mode 100644 tests/components/DraftsCard.test.tsx create mode 100644 tests/unit/entwuerfe.test.ts diff --git a/actions/hireDrafts.ts b/actions/hireDrafts.ts index a60a9c0..130eab6 100644 --- a/actions/hireDrafts.ts +++ b/actions/hireDrafts.ts @@ -16,13 +16,22 @@ 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 - // Policy hire_drafts_owner — nicht eine Prüfung hier. - await tx + // Ob die Zeile der aufrufenden Person gehört, entscheidet die Regel + // hire_drafts_update — nicht eine Prüfung hier. + // + // 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. + const ergebnis = await tx .updateTable("hire_drafts") .set({ step: payload.step, payload: payload.data, updated_at: new Date().toISOString() }) .where("id", "=", payload.id) - .execute(); + .executeTakeFirst(); + if (ergebnis.numUpdatedRows === 0n) { + throw new Error("Der Entwurf liess sich nicht speichern: er gehört jemand anderem oder wurde inzwischen gelöscht."); + } return payload.id; } @@ -43,7 +52,15 @@ export async function saveHireDraft(payload: { export async function deleteHireDraft(id: string): Promise { try { - await withUser(await currentUserId(), (tx) => tx.deleteFrom("hire_drafts").where("id", "=", id).execute()); + // 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. + 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."); + } + }); revalidatePath("/"); return { success: true }; } catch (err) { diff --git a/actions/notes.ts b/actions/notes.ts index b474824..1b1962f 100644 --- a/actions/notes.ts +++ b/actions/notes.ts @@ -6,7 +6,12 @@ import { withUser } from "@/lib/db"; import type { ActionResult } from "@/lib/db/rpc"; /** - * Notizen einer Kollegin oder eines Kollegen hinzuwählen oder abwählen. + * Eine Kollegin oder einen Kollegen hinzuwählen oder abwählen. + * + * Die Auswahl steuert zwei Listen: die Notizen in der Glocke und die + * Entwürfe auf der Übersicht. Der Name der Funktion nennt nur die erste, + * weil die Einstellung in der Glocke sitzt — was sie bewirkt, steht an der + * Einstellung selbst. * * Kein Aufruf einer SQL-Funktion und kein Protokolleintrag, anders als bei * allem, was Personaldaten ändert: das hier ist eine persönliche @@ -15,7 +20,7 @@ import type { ActionResult } from "@/lib/db/rpc"; * Dasselbe Muster wie bei gespeicherten Auswertungen und Entwürfen * (actions/reports.ts, actions/hireDrafts.ts). * - * Abgesichert ist es trotzdem: die Regel `note_subscriptions_owner` lässt nur Zeilen + * Abgesichert ist es trotzdem: die Regel `colleague_subscriptions_owner` lässt nur Zeilen * zu, deren `user_id` die angemeldete Person ist. Eine fremde Einstellung * liesse sich auch mit erfundenen Werten nicht schreiben. */ @@ -38,13 +43,13 @@ export async function setNotizSichtbarkeit(payload: { // `on conflict do nothing`: zweimal dasselbe Hinzuwählen ist kein // Fehler, sondern derselbe Wunsch — etwa wenn zwei Reiter offen sind. await tx - .insertInto("note_subscriptions") + .insertInto("colleague_subscriptions") .values({ user_id: userId, author_user_id: payload.kollegeId }) .onConflict((oc) => oc.columns(["user_id", "author_user_id"]).doNothing()) .execute(); } else { await tx - .deleteFrom("note_subscriptions") + .deleteFrom("colleague_subscriptions") .where("user_id", "=", userId) .where("author_user_id", "=", payload.kollegeId) .execute(); diff --git a/app/(app)/page.tsx b/app/(app)/page.tsx index 3c79862..46251a1 100644 --- a/app/(app)/page.tsx +++ b/app/(app)/page.tsx @@ -183,7 +183,7 @@ export default async function DashboardPage({ return (
- {drafts && drafts.length > 0 && } + {drafts.length > 0 && }
{kpis.map((kpi) => ( ; updated_at: string }; +// 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. -export function DraftsCard({ drafts }: { drafts: Draft[] }) { +export function DraftsCard({ drafts }: { drafts: Entwurf[] }) { const { openWizard } = useHireWizard(); const { showToast } = useToast(); const router = useRouter(); @@ -39,25 +46,38 @@ export function DraftsCard({ drafts }: { drafts: Draft[] }) { const lastName = typeof d.payload.lastName === "string" ? d.payload.lastName : ""; const name = [firstName, lastName].filter(Boolean).join(" ") || "Ohne Namen"; return ( -
  • +
  • {name} Gespeichert am {fmtDate(d.updated_at)} + {/* Nur bei fremden Entwürfen. Der eigene Name stünde sonst an + jeder Zeile und sagte nichts. */} + {d.autor && von {d.autor}}
    -
    - - -
    + {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/shell/NotesBell.tsx b/components/shell/NotesBell.tsx index 71bd072..9c16428 100644 --- a/components/shell/NotesBell.tsx +++ b/components/shell/NotesBell.tsx @@ -99,8 +99,13 @@ export function NotesBell({ notes, kollegen }: { notes: OpenNote[]; kollegen: Ko {zeigeEinstellung && (
    + {/* Dass die Auswahl auch die Entwürfe auf der Übersicht + steuert, steht hier und nicht nur dort: sie wird hier + getroffen, und eine Wirkung, die zwei Klicks entfernt + sichtbar wird, sucht sonst niemand. */}

    - Wessen Notizen hier zusätzlich erscheinen. Die eigenen sind immer dabei. + Wessen Notizen hier und wessen Entwürfe auf der Übersicht zusätzlich erscheinen. Die + eigenen sind immer dabei.

    {kollegen.length === 0 ? (

    Keine weiteren HR-Kolleg:innen freigeschaltet.

    diff --git a/db/migrations/20260910120000_abos_gelten_auch_fuer_entwuerfe.sql b/db/migrations/20260910120000_abos_gelten_auch_fuer_entwuerfe.sql new file mode 100644 index 0000000..6471a8d --- /dev/null +++ b/db/migrations/20260910120000_abos_gelten_auch_fuer_entwuerfe.sql @@ -0,0 +1,118 @@ +-- Die Auswahl der Kolleg:innen gilt jetzt für Notizen **und** Entwürfe. +-- +-- ═══ 1. Der Name stimmt nicht mehr ═══════════════════════════════ +-- +-- `note_subscriptions` hiess nach dem, wofür die Auswahl gestern allein +-- galt. Sie steuert ab jetzt zwei Listen; ein Name, der nur eine davon +-- nennt, führt beim nächsten Lesen in die Irre. + +alter table note_subscriptions rename to colleague_subscriptions; +alter index idx_note_subscriptions_user rename to idx_colleague_subscriptions_user; +alter table colleague_subscriptions + rename constraint chk_note_abos_nicht_selbst to chk_kollegen_abos_nicht_selbst; + +drop policy if exists "note_subscriptions_owner" on colleague_subscriptions; +drop policy if exists "colleague_subscriptions_owner" on colleague_subscriptions; +create policy "colleague_subscriptions_owner" on colleague_subscriptions + for all + using (user_id = app_current_user_id() and is_hr_user()) + with check (user_id = app_current_user_id() and is_hr_user()); + +comment on table colleague_subscriptions is + 'Hinzugewählte Kolleg:innen je Person. Eine Zeile holt deren Notizen und Entwürfe in die Ansicht von user_id. Ohne Zeile sieht man nur die eigenen.'; + +-- ═══ 2. Entwürfe: lesen weiter, schreiben eng ════════════════════ +-- +-- Bisher stand über hire_drafts eine einzige Regel `for all`. Sie wird in +-- vier zerlegt, weil Lesen und Schreiben ab jetzt verschieden weit reichen: +-- +-- lesen — die eigenen und die der hinzugewählten Kolleg:innen +-- anlegen — nur auf den eigenen Namen +-- ändern — nur die eigenen +-- löschen — nur die eigenen +-- +-- Warum das Schreiben eng bleibt: ein Entwurf ist unfertige Arbeit. Zwei +-- Personen, die abwechselnd in derselben Zeile schreiben, überschreiben +-- einander lautlos — es gibt keine Sperre und keine Historie, die das +-- auffangen könnte. Wer einen fremden Entwurf übernehmen soll, braucht dafür +-- einen eigenen Vorgang, keine stillschweigend geöffnete Regel. +-- +-- ═══ Was das erzwingt ═══ +-- +-- Solange fremde Entwürfe unsichtbar waren, konnte niemand versuchen, einen +-- zu speichern. Jetzt schon — und ein UPDATE, das die Regel abweist, trifft +-- keine Zeile und meldet trotzdem keinen Fehler. Die Anwendung muss die Zahl +-- der geänderten Zeilen prüfen (actions/hireDrafts.ts), sonst sähe die +-- Person „gespeichert", und nichts wäre gespeichert. + +drop policy if exists "hire_drafts_owner" on hire_drafts; + +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 ( + created_by = 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 = hire_drafts.created_by + ) + ) + ); + +drop policy if exists "hire_drafts_insert" on hire_drafts; +create policy "hire_drafts_insert" on hire_drafts + for insert + with check (created_by = app_current_user_id() and is_hr_user()); + +drop policy if exists "hire_drafts_update" on hire_drafts; +create policy "hire_drafts_update" on hire_drafts + for update + using (created_by = app_current_user_id() and is_hr_user()) + with check (created_by = app_current_user_id() and is_hr_user()); + +drop policy if exists "hire_drafts_delete" on hire_drafts; +create policy "hire_drafts_delete" on hire_drafts + for delete + using (created_by = app_current_user_id() and is_hr_user()); + +-- ═══ Gegenprobe ═══════════════════════════════════════════════════ +do $$ +declare + anzahl int; +begin + if exists (select 1 from pg_tables where tablename = 'note_subscriptions') then + raise exception 'note_subscriptions steht noch — die Umbenennung hat nicht gegriffen.'; + end if; + + if not exists ( + select 1 from pg_policies + where tablename = 'colleague_subscriptions' and policyname = 'colleague_subscriptions_owner' + ) then + raise exception 'Die Eigentuemerregel auf colleague_subscriptions fehlt.'; + end if; + + -- Die alte Sammelregel darf nicht übrigbleiben: sie erlaubte `for all` + -- und machte die vier neuen wirkungslos, weil Policies sich addieren. + if exists (select 1 from pg_policies where tablename = 'hire_drafts' and policyname = 'hire_drafts_owner') then + raise exception 'hire_drafts_owner steht noch — die alte Sammelregel wuerde die neuen aushebeln.'; + 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 (select, insert, update, delete).', anzahl; + end if; + + -- Keine der drei Schreibregeln darf die Abos kennen. Stünde dort dieselbe + -- Bedingung wie beim Lesen, liesse sich ein fremder Entwurf überschreiben. + if exists ( + select 1 from pg_policies + where tablename = 'hire_drafts' + and cmd in ('INSERT', 'UPDATE', 'DELETE') + and coalesce(qual, '') || coalesce(with_check, '') like '%colleague_subscriptions%' + ) then + raise exception 'Eine Schreibregel auf hire_drafts kennt die Abos — Schreiben muss eigentuemergebunden bleiben.'; + end if; +end $$; diff --git a/docs/datenkatalog.md b/docs/datenkatalog.md index 1d4acbf..443b7fb 100644 --- a/docs/datenkatalog.md +++ b/docs/datenkatalog.md @@ -268,8 +268,30 @@ ist ein bewusster zweiter Schritt. ### `hire_drafts`, `saved_reports` Zwischenstand des Einstellungsassistenten (`payload` JSONB, `step`) und -gespeicherte Berichtskonfigurationen. Beide sind auf die anlegende Person -eingeschränkt. +gespeicherte Berichtskonfigurationen. + +`saved_reports` gehört ganz der anlegenden Person. Bei `hire_drafts` gehen +Lesen und Schreiben seit September 2026 verschieden weit: **lesen** darf, wem +der Entwurf gehört, und wer die anlegende Person in seiner Glocke hinzugewählt +hat (`colleague_subscriptions`); **anlegen, ändern und löschen** darf nur die +anlegende Person selbst. Ein Entwurf ist unfertige Arbeit ohne Sperre und ohne +Historie — zwei Personen, die abwechselnd hineinschreiben, überschreiben +einander lautlos. + +Die Anwendung liest deshalb bei `update` und `delete` die Zahl der +betroffenen Zeilen: eine Policy weist ein Schreiben nicht mit einem Fehler ab, +sie lässt es ins Leere laufen (`actions/hireDrafts.ts`). + +### `colleague_subscriptions` + +Wen jemand hinzugewählt hat, je eine Zeile aus `user_id` und +`author_user_id`. Die Vorgabe ist die eigene Person: **ohne** Zeile sieht man +nur die eigenen Notizen und Entwürfe, mit Zeile zusätzlich die der genannten +Person. Die Auswahl sitzt in der Glocke in der Kopfzeile und steuert beides. + +Ein Abo auf sich selbst lässt `chk_kollegen_abos_nicht_selbst` nicht zu — die +eigenen Zeilen sind ohnehin immer dabei. Die Tabelle hiess bis September 2026 +`note_subscriptions`, als die Auswahl nur für die Notizen galt. --- @@ -390,8 +412,12 @@ Wo das Muster abweicht: ohne Freischaltung. Sonst könnte niemand erfahren, warum er nicht hineinkommt. - `app_users` ebenso: die eigene Zeile oder HR. -- `hire_drafts` und `saved_reports` verlangen zusätzlich, dass die Zeile der - anfragenden Person gehört. +- `saved_reports` und `colleague_subscriptions` verlangen zusätzlich, dass + die Zeile der anfragenden Person gehört. +- `hire_drafts` hat vier — `select`, `insert`, `update`, `delete` getrennt, + weil das Lesen seit September 2026 weiter reicht als das Schreiben (siehe + oben). Die alte Sammelregel `for all` musste dafür weichen: Policies + addieren sich, eine stehengebliebene hätte die vier neuen ausgehebelt. - `locations` trennt Lesen und Schreiben in zwei Policies, prüft aber beide Male dasselbe. diff --git a/lib/dashboard-data.ts b/lib/dashboard-data.ts index a94a55b..593b526 100644 --- a/lib/dashboard-data.ts +++ b/lib/dashboard-data.ts @@ -1,7 +1,8 @@ import type { Tx } from "./db"; -import { jsonArrayFrom, jsonObjectFrom, zeitstempel } from "./db/json"; +import { jsonArrayFrom, jsonObjectFrom } from "./db/json"; import { besetzungenAbfrage, pickPlacements } from "./placement"; import { buildOrgMaps, orgMapsAbfragen, type OrgEb } from "./org"; +import { baueEntwuerfe, entwuerfeAbfrage, type EntwurfZeile } from "./entwuerfe"; import { sichtbareNotizen } from "./notes"; import { offeneStellenAbfrage, resolveOpenPositions, type OffeneStelle } from "./positions"; import type { HistoryEventType } from "./types"; @@ -13,6 +14,9 @@ import type { AnstehendArt } from "./dashboard-filter"; // das Ergebnis gegen den alten Weg halten lässt, ohne eine React-Komponente // aufzubauen. +/** Eine gültige uuid, die keiner Zeile gehört — für den abgemeldeten Fall. */ +const KEINE_KENNUNG = "00000000-0000-0000-0000-000000000000"; + export type DashboardParams = { userId: string | null; today: string; @@ -42,14 +46,14 @@ export async function loadDashboardData(tx: Tx, p: DashboardParams) { const g = await tx .selectNoFrom((eb) => [ - jsonArrayFrom( - eb - .selectFrom("hire_drafts") - .select(["id", "step", "payload"]) - .select((x) => zeitstempel(x.ref("updated_at")).as("updated_at")) - .where("created_by", "=", userId ?? "") - .orderBy("updated_at", "desc") - ).as("drafts"), + // Die eigenen Entwürfe und die der hinzugewählten Kolleg:innen. + // + // Ohne Anmeldung geht eine Kennung mit, die es nicht gibt: die Spalte + // ist vom Typ uuid, und der leere Text, der hier stand, hätte sich + // nicht dorthin umwandeln lassen — die Abfrage wäre nicht leer + // ausgegangen, sondern gescheitert. Das Ergebnis wird unten ohnehin + // verworfen. + jsonArrayFrom(entwuerfeAbfrage(eb, userId ?? KEINE_KENNUNG)).as("drafts"), jsonArrayFrom( eb @@ -148,7 +152,7 @@ export async function loadDashboardData(tx: Tx, p: DashboardParams) { const orgMaps = buildOrgMaps(g.units as never, g.locations as never); return { - drafts: userId ? g.drafts : [], + drafts: userId ? baueEntwuerfe(g.drafts as EntwurfZeile[], userId) : [], staffRows: g.staffRows, hiresYtd: Number(g.hiresYtd?.anzahl ?? 0), exitsYtd: Number(g.exitsYtd?.anzahl ?? 0), diff --git a/lib/entwuerfe.ts b/lib/entwuerfe.ts new file mode 100644 index 0000000..9f67dbb --- /dev/null +++ b/lib/entwuerfe.ts @@ -0,0 +1,84 @@ +import { sql } from "kysely"; +import { zeitstempel } from "./db/json"; +import { istHinzugewaehlt } from "./kollegen"; +import type { OrgEb } from "./org"; + +// Die angefangenen Neueinstellungen auf der Übersicht. +// +// Bis September 2026 sah jede Person nur die eigenen. Seit die Auswahl in der +// Glocke auch hierfür gilt (lib/kollegen.ts), kommen die Entwürfe der +// hinzugewählten Kolleg:innen dazu — damit eine angefangene Einstellung nicht +// liegen bleibt, weil die eine Person im Urlaub ist und die andere nicht weiss, +// dass es sie gibt. +// +// **Nur ansehen.** Fortsetzen und Löschen bleiben bei der Person, von der der +// Entwurf stammt; die Regeln hire_drafts_update und hire_drafts_delete lassen +// nichts anderes zu. Ein Entwurf ist unfertige Arbeit ohne Sperre und ohne +// Historie — zwei Personen, die abwechselnd hineinschreiben, überschreiben +// einander lautlos. + +export type EntwurfZeile = { + id: string; + step: number; + payload: Record; + updated_at: string; + created_by: string | null; + author_name: string | null; + author_email: string | null; +}; + +export type Entwurf = { + id: string; + step: number; + payload: Record; + updated_at: string; + /** Ob der Entwurf von der angemeldeten Person stammt — nur dann darf sie ihn anfassen. */ + vonMir: boolean; + /** Wer ihn angefangen hat. Leer bei den eigenen: der Name sagte dort nichts. */ + autor: string | null; +}; + +/** + * Die Entwürfe als *Teilabfrage* — zum Einhängen in die eine Abfrage, die die + * Übersicht ohnehin stellt (lib/db/json.ts). + * + * Die Bedingung wiederholt, was `hire_drafts_select` ohnehin durchlässt. Das + * ist Absicht: die Regel in der Datenbank ist die Grenze, aber sie steht in + * einer Migration und lässt sich von hier aus nicht lesen. Steht sie auch in + * der Abfrage, sagt der Quelltext, was die Liste zeigt — und ein Test kann es + * festhalten. Fällt eine der beiden aus, bleibt die andere. + * + * `created_by` kann leer sein (die Person wurde gelöscht). Solche Zeilen zeigt + * niemand: anders als bei einer Notiz steht in einem Entwurf nichts, was ohne + * die Person, die ihn angefangen hat, noch weiterginge. + */ +export function entwuerfeAbfrage(eb: OrgEb, userId: string) { + return eb + .selectFrom("hire_drafts as d") + .leftJoin("profiles as p", "p.id", "d.created_by") + .select(["d.id", "d.step", "d.payload", "d.created_by"]) + .select(["p.full_name as author_name", "p.email as author_email"]) + .select((x) => zeitstempel(x.ref("d.updated_at")).as("updated_at")) + .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. + .orderBy(sql`case when "d"."created_by" = ${userId} then 0 else 1 end`) + .orderBy("d.updated_at", "desc"); +} + +/** Der reine Teil: aus den Zeilen die Entwürfe mit lesbarem Verfasser. */ +export function baueEntwuerfe(rows: EntwurfZeile[], userId: string | null): Entwurf[] { + return rows.map((row) => { + const vonMir = userId !== null && row.created_by === userId; + return { + id: row.id, + step: row.step, + payload: row.payload, + updated_at: row.updated_at, + vonMir, + // Ohne Namen die E-Mail — sonst stünde an einem fremden Entwurf nichts + // ausser dem Hinweis, dass er fremd ist. + autor: vonMir ? null : row.author_name?.trim() || row.author_email || "Unbekannt", + }; + }); +} diff --git a/lib/kollegen.ts b/lib/kollegen.ts new file mode 100644 index 0000000..0d5aef4 --- /dev/null +++ b/lib/kollegen.ts @@ -0,0 +1,28 @@ +import { sql, type Expression, type SqlBool } from "kysely"; + +// Wen jemand hinzugewählt hat — die eine Regel hinter zwei Listen. +// +// Bis September 2026 galt die Auswahl nur für die Notizen in der Glocke; sie +// hiess deshalb `note_subscriptions` und die Bedingung stand in lib/notes.ts. +// Seit die Entwürfe derselben Auswahl folgen, steht sie hier: zwei Fassungen +// derselben Regel driften auseinander, und dann zeigte die eine Liste +// jemanden, den die andere nicht kennt. + +/** + * Ob `spalte` auf eine hinzugewählte Person zeigt. + * + * `spalte` ist der Name der Verfasserspalte in der umgebenden Abfrage + * (`n.author_user_id`, `d.created_by`, …) — als Bezeichner eingesetzt, nicht + * als Wert. Er kommt aus dem Quelltext, nie aus einer Eingabe; die Kennung + * dagegen kommt aus der Sitzung und geht als Parameter mit. + * + * Die eigene Person steht bewusst **nicht** darin. Wer sich selbst sieht, ist + * keine Frage der Auswahl, sondern die Vorgabe — und die Prüfbedingung + * `chk_kollegen_abos_nicht_selbst` lässt ein Abo auf sich selbst gar nicht zu. + * Die aufrufende Abfrage stellt „die eigenen" davor, mit ODER. + */ +export function istHinzugewaehlt(userId: string, spalte: string): Expression { + return sql`exists ( + select 1 from colleague_subscriptions s + where s.user_id = ${userId} and s.author_user_id = ${sql.ref(spalte)})`; +} diff --git a/lib/notes.ts b/lib/notes.ts index c63e536..1a897b4 100644 --- a/lib/notes.ts +++ b/lib/notes.ts @@ -2,6 +2,7 @@ import { sql, type Expression, type SqlBool } from "kysely"; import type { Tx } from "./db"; import { jsonArrayFrom, zeitstempel } from "./db/json"; import { fmtName } from "./format"; +import { istHinzugewaehlt } from "./kollegen"; import type { OrgEb } from "./org"; import type { Database } from "./types"; @@ -29,7 +30,8 @@ export type NotizZeile = Omit`( ${verfasser} is null or ${verfasser} = ${userId} - or exists ( - select 1 from note_subscriptions s - where s.user_id = ${userId} and s.author_user_id = ${verfasser}))`; + or ${istHinzugewaehlt(userId, spalte)})`; } /** diff --git a/lib/shell-data.ts b/lib/shell-data.ts index ec73a10..7c543fd 100644 --- a/lib/shell-data.ts +++ b/lib/shell-data.ts @@ -33,7 +33,7 @@ export type ShellData = { * * `sichtbar` ist der Stand des Hakens: gesetzt, wenn die Person * hinzugewählt ist. Die eigene Person steht nicht in der Liste — die - * eigenen Notizen sind immer dabei. + * eigenen Notizen und Entwürfe sind immer dabei. */ kollegen: { id: string; name: string; sichtbar: boolean }[]; }; @@ -75,7 +75,7 @@ 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"), diff --git a/lib/types.ts b/lib/types.ts index 1e410b9..9a9e233 100644 --- a/lib/types.ts +++ b/lib/types.ts @@ -577,8 +577,8 @@ export type Database = { // Einbettung erspart an einem Dutzend Stellen eine zweite Abfrage. }; // Hinzugewählte Kolleg:innen je Person. Eine Zeile holt deren Notizen - // in die Glocke; ohne Zeile sieht man nur die eigenen. - note_subscriptions: { + // und Entwürfe in die Ansicht; ohne Zeile sieht man nur die eigenen. + colleague_subscriptions: { Row: { user_id: string; author_user_id: string; @@ -589,7 +589,7 @@ export type Database = { author_user_id: string; created_at?: string; }; - Update: Partial; + Update: Partial; }; }; Views: Record; diff --git a/scripts/check-schema-types.mjs b/scripts/check-schema-types.mjs index 6d65099..a6f7ca9 100644 --- a/scripts/check-schema-types.mjs +++ b/scripts/check-schema-types.mjs @@ -114,6 +114,19 @@ function parseMigrations() { } const anweisung = sql.slice(kopf.index + kopf[0].length, ende); + // Eine umbenannte Tabelle behält ihre Spalten und wechselt den Namen. + // Ohne diesen Fall stünde der alte Name weiter im Modell aus den + // Migrationen, und der Abgleich meldete zwei Fehler für eine Änderung: + // die alte Tabelle fehle in lib/types.ts, die neue kenne das Schema + // nicht. `rename constraint` und `rename column` sind davon nicht + // betroffen — nur das unmittelbare `rename to`. + const umbenannt = anweisung.match(/^\s*rename to (\w+)/i); + if (umbenannt) { + tables.delete(kopf[1].toLowerCase()); + tables.set(umbenannt[1].toLowerCase(), tabelle); + continue; + } + for (const m of anweisung.matchAll(/\badd column (?:if not exists )?(\w+)/gi)) { tabelle.add(m[1].toLowerCase()); } diff --git a/tests/components/DraftsCard.test.tsx b/tests/components/DraftsCard.test.tsx new file mode 100644 index 0000000..0fb18b0 --- /dev/null +++ b/tests/components/DraftsCard.test.tsx @@ -0,0 +1,134 @@ +import { render, screen, within } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { DraftsCard } from "@/components/dashboard/DraftsCard"; +import { ToastProvider } from "@/components/ui/Toast"; +import type { Entwurf } from "@/lib/entwuerfe"; + +const openWizard = vi.fn(); +const deleteHireDraft = vi.fn<(id: string) => Promise<{ success: boolean; error?: string }>>(async () => ({ + success: true, +})); +const refresh = vi.fn(); + +vi.mock("@/components/hire/HireWizardContext", () => ({ useHireWizard: () => ({ openWizard }) })); +vi.mock("@/actions/hireDrafts", () => ({ deleteHireDraft: (id: string) => deleteHireDraft(id) })); +vi.mock("next/navigation", () => ({ useRouter: () => ({ refresh, push: vi.fn() }) })); + +// Seit die Auswahl in der Glocke auch die Entwürfe steuert, stehen hier +// fremde Zeilen. Sie dürfen aussehen wie das, was sie sind: zum Ansehen. +// Böte die Karte an ihnen „Fortsetzen", liefe die Person durch den ganzen +// Assistenten und bekäme am Ende eine Fehlermeldung aus der Datenbank. + +function entwurf(teil: Partial = {}): Entwurf { + return { + id: "d1", + step: 2, + payload: { firstName: "Manuel", lastName: "Aigner" }, + updated_at: "2026-09-09T08:00:00.000Z", + vonMir: true, + autor: null, + ...teil, + }; +} + +const fremd = entwurf({ id: "d2", payload: { firstName: "Eva", lastName: "Novak" }, vonMir: false, autor: "Anna Berger" }); + +function zeige(drafts: Entwurf[]) { + return render( + + + + ); +} + +/** Die Zeile zu einem Namen — die Karte zeigt mehrere nebeneinander. */ +function zeile(name: string) { + return screen.getByText(name).closest("li") as HTMLElement; +} + +beforeEach(() => { + openWizard.mockClear(); + deleteHireDraft.mockClear(); + deleteHireDraft.mockResolvedValue({ success: true }); + refresh.mockClear(); +}); + +describe("DraftsCard", () => { + it("zeigt zu eigenen Entwürfen Fortsetzen und Löschen", () => { + zeige([entwurf()]); + const z = zeile("Manuel Aigner"); + expect(within(z).getByRole("button", { name: "Fortsetzen" })).toBeInTheDocument(); + expect(within(z).getByRole("button", { name: "Entwurf löschen" })).toBeInTheDocument(); + }); + + it("nennt an eigenen Entwürfen keinen Verfasser", () => { + // Der eigene Name stünde an jeder Zeile und sagte nichts. + zeige([entwurf()]); + expect(within(zeile("Manuel Aigner")).queryByText(/^von /)).not.toBeInTheDocument(); + }); + + it("nennt an fremden Entwürfen, von wem sie stammen", () => { + zeige([fremd]); + 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. + 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(); + }); + + 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. + zeige([fremd]); + expect(within(zeile("Eva Novak")).getByText("Nur zur Ansicht")).toBeInTheDocument(); + }); + + it("trennt die Zeilen: eigene bedienbar, fremde nicht", () => { + // Beide nebeneinander — der Fall, in dem eine Verwechslung teuer wird. + zeige([entwurf(), fremd]); + expect(within(zeile("Manuel Aigner")).getByRole("button", { name: "Fortsetzen" })).toBeInTheDocument(); + expect(within(zeile("Eva Novak")).queryByRole("button", { name: "Fortsetzen" })).not.toBeInTheDocument(); + }); + + it("setzt beim Klick genau den angeklickten Entwurf fort", () => { + const user = userEvent.setup(); + zeige([entwurf(), fremd]); + return user.click(within(zeile("Manuel Aigner")).getByRole("button", { name: "Fortsetzen" })).then(() => { + expect(openWizard).toHaveBeenCalledWith({ draftId: "d1" }); + }); + }); + + it("löscht genau den angeklickten Entwurf", async () => { + // Zwei eigene nebeneinander: eine Karte, die immer den ersten löscht, + // fiele mit nur einer Zeile nicht auf. + const user = userEvent.setup(); + zeige([entwurf(), entwurf({ id: "d3", payload: { firstName: "Lena", lastName: "Wolf" } })]); + await user.click(within(zeile("Lena Wolf")).getByRole("button", { name: "Entwurf löschen" })); + expect(deleteHireDraft).toHaveBeenCalledWith("d3"); + }); + + it("meldet den Fehler, wenn das Löschen abgewiesen wird", async () => { + deleteHireDraft.mockResolvedValue({ success: false, error: "Gehört jemand anderem." }); + const user = userEvent.setup(); + zeige([entwurf()]); + await user.click(screen.getByRole("button", { name: "Entwurf löschen" })); + expect(await screen.findByText("Gehört jemand anderem.")).toBeInTheDocument(); + }); + + it("bleibt bei leerer Liste unsichtbar", () => { + // Eine Überschrift über nichts ist eine Zeile, die jeden Tag gelesen und + // jeden Tag verworfen wird. + // Nicht auf einen leeren Container prüfen: der ToastProvider legt + // seinen eigenen Kasten an, und der stünde da, egal was die Karte tut. + zeige([]); + expect(screen.queryByRole("heading", { name: /Entwürfe/ })).not.toBeInTheDocument(); + expect(screen.queryByRole("list")).not.toBeInTheDocument(); + }); +}); diff --git a/tests/unit/entwuerfe.test.ts b/tests/unit/entwuerfe.test.ts new file mode 100644 index 0000000..b403340 --- /dev/null +++ b/tests/unit/entwuerfe.test.ts @@ -0,0 +1,136 @@ +import { DummyDriver, Kysely, PostgresAdapter, PostgresIntrospector, PostgresQueryCompiler } from "kysely"; +import { describe, expect, it } from "vitest"; +import type { Schema } from "@/lib/db/schema"; +import { baueEntwuerfe, entwuerfeAbfrage, type EntwurfZeile } from "@/lib/entwuerfe"; + +// Wessen Entwürfe jemand sieht — und, wichtiger, an welchen die Schaltflächen +// hängen. Die Regeln in der Datenbank sind die Grenze; hier wird gelesen, was +// die Abfrage daraus macht, weil ein Fehler darin nicht auffiele: die Karte +// zeigte eine Liste, nur die falsche. + +const db = new Kysely({ + dialect: { + createAdapter: () => new PostgresAdapter(), + createDriver: () => new DummyDriver(), + createIntrospector: (d) => new PostgresIntrospector(d), + createQueryCompiler: () => new PostgresQueryCompiler(), + }, +}); + +const ICH = "11111111-1111-1111-1111-111111111111"; +const ANDERE = "22222222-2222-2222-2222-222222222222"; + +// entwuerfeAbfrage erwartet den Ausdrucksbauer aus selectNoFrom — genau so +// hängt lib/dashboard-data.ts sie ein. +function abfrage(userId = ICH) { + return db.selectNoFrom((eb) => [entwuerfeAbfrage(eb, userId).as("x")]).compile(); +} + +const text = (userId?: string) => abfrage(userId).sql.replace(/\s+/g, " "); + +describe("entwuerfeAbfrage", () => { + it("zeigt die eigenen Entwürfe", () => { + expect(text()).toContain('"d"."created_by" = $'); + }); + + it("holt dazu, wer hinzugewählt wurde", () => { + // Vorzeichen und Tabelle zusammen: ein `not exists` kehrte die Bedeutung + // um, ohne dass ein Wort sich änderte. + const s = text(); + expect(s).toContain("exists"); + expect(s).not.toContain("not exists"); + expect(s).toContain("from colleague_subscriptions s"); + expect(s).toContain('s.author_user_id = "d"."created_by"'); + }); + + it("verknüpft eigene und hinzugewählte mit ODER, nicht mit UND", () => { + // Mit UND bliebe die Liste immer leer: kein Entwurf ist gleichzeitig von + // mir und von jemandem, den ich hinzugewählt habe. + expect(text()).toMatch(/"d"\."created_by" = \$\d+ or exists/); + }); + + it("holt den Namen der verfassenden Person dazu", () => { + // Ohne ihn stünde an einem fremden Entwurf nur, dass er fremd ist. + const s = text(); + expect(s).toContain('left join "profiles" as "p" on "p"."id" = "d"."created_by"'); + expect(s).toContain('"p"."full_name" as "author_name"'); + expect(s).toContain('"p"."email" as "author_email"'); + }); + + it("stellt die eigenen Entwürfe nach vorn", () => { + // Vor der Freigabe standen dort nur die eigenen; wer seinen halbfertigen + // Entwurf sucht, soll ihn nicht zwischen fremden suchen. + const s = text(); + expect(s).toMatch(/order by case when "d"\."created_by" = \$\d+ then 0 else 1 end/); + expect(s).toContain('"d"."updated_at" desc'); + }); + + it("bindet die Kennung als Parameter, nicht in den Text", () => { + const { sql: roh, parameters } = abfrage("bösartig'; drop table hire_drafts; --"); + expect(roh).not.toContain("drop table"); + expect(parameters).toContain("bösartig'; drop table hire_drafts; --"); + }); + + 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. + expect(text()).toContain("to_char("); + }); +}); + +describe("baueEntwuerfe", () => { + function zeile(teil: Partial = {}): EntwurfZeile { + return { + id: "d1", + step: 2, + payload: { firstName: "Manuel", lastName: "Aigner" }, + updated_at: "2026-09-09T08:00:00.000Z", + created_by: ANDERE, + author_name: "Anna Berger", + author_email: "a@example.test", + ...teil, + }; + } + + it("erkennt den eigenen Entwurf", () => { + const [e] = baueEntwuerfe([zeile({ created_by: ICH })], ICH); + expect(e.vonMir).toBe(true); + }); + + it("nennt bei eigenen Entwürfen keinen Verfasser", () => { + // Der eigene Name stünde an jeder Zeile und sagte nichts. + const [e] = baueEntwuerfe([zeile({ created_by: ICH })], ICH); + expect(e.autor).toBeNull(); + }); + + it("nennt bei fremden Entwürfen den Namen", () => { + const [e] = baueEntwuerfe([zeile()], ICH); + expect(e.vonMir).toBe(false); + expect(e.autor).toBe("Anna Berger"); + }); + + it("fällt ohne Namen auf die E-Mail zurück", () => { + // Auch ein Name aus Leerzeichen zählt als keiner. + expect(baueEntwuerfe([zeile({ author_name: " " })], ICH)[0].autor).toBe("a@example.test"); + expect(baueEntwuerfe([zeile({ author_name: null })], ICH)[0].autor).toBe("a@example.test"); + }); + + it("nennt jemanden, auch wenn beides fehlt", () => { + // Ein Entwurf ohne jede Zuordnung wäre einer, bei dem niemand weiss, wen + // er fragen soll. + expect(baueEntwuerfe([zeile({ author_name: null, author_email: null })], ICH)[0].autor).toBe("Unbekannt"); + }); + + it("hält ohne Anmeldung nichts für eigen", () => { + // created_by kann leer sein, userId auch. Beide leer heisst nicht „meiner" + // — sonst hinge an einer herrenlosen Zeile eine Löschen-Schaltfläche. + const [e] = baueEntwuerfe([zeile({ created_by: null })], null); + expect(e.vonMir).toBe(false); + }); + + 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" }); + expect(e.payload).toEqual({ firstName: "Manuel", lastName: "Aigner" }); + }); +}); diff --git a/tests/unit/notes-visibility.test.ts b/tests/unit/notes-visibility.test.ts index 4a8f399..3599cdf 100644 --- a/tests/unit/notes-visibility.test.ts +++ b/tests/unit/notes-visibility.test.ts @@ -48,7 +48,7 @@ describe("sichtbareNotizen", () => { const s = sql(); expect(s).toContain("exists"); expect(s).not.toContain("not exists"); - expect(s).toContain("from note_subscriptions s"); + expect(s).toContain("from colleague_subscriptions s"); expect(s).toContain("s.user_id = $"); expect(s).toContain("s.author_user_id = \"n\".\"author_user_id\""); });