Stop offering positions that are already spoken for, and let hiring work again

Two reports, four defects, all of them in the way of ordinary use.

A position with a signed starter is not vacant. loadOpenPositions asked "is
anyone on it today?", so three positions whose new holders begin in
September and October were listed as open, labelled "vacant for 2 days".
That same list feeds the hire wizard, so it invited filling a position a
second time — discovered at the partial unique index, after the second
interview. Vacancy now means no assignment that still stands, including one
that has not started. An assignment that ended still frees the position.

Hiring was broken three times over, each fault hidden behind the previous
one:

  1. hire_employee cast to ::weekday[], a type that no longer exists — it
     was replaced by text plus a CHECK constraint and the function was never
     updated. apply_due_pending_changes had the same problem with
     ::relationship_type, which would have broken the nightly run.
     PL/pgSQL resolves types in embedded statements at execution time, so
     both functions were created without complaint and failed only in use.
  2. Fourteen functions called auth.uid(). The application connects as a
     role with no rights on the auth schema, so every write — hire,
     transfer, promote, exit, notes, positions — failed with "permission
     denied for schema auth". They now use app_current_user_id(), which is
     where #23 was heading anyway. Its own fallback also caught only
     "function missing" and now catches the privilege error too, so a call
     without session context returns null instead of raising.
  3. The audit line built a name as `payload->>'a' || ' ' || payload->>'b'`.
     `||` binds tighter than `->>`, so Postgres reads
     `payload ->> ('a' || ' ' || payload) ->> 'b'`. The ACL failure above
     had aborted analysis before the parser ever reached it.

And the wizard collected an email, showed it in the summary, and dropped it:
the server action's signature had no such field. employees.email is NOT
NULL, so every hire that got past the three faults above would have failed
there. It is now passed through and required in step one, rather than
refused by the database at the end of step four.

Verified against the live database, each rolled back: a hire now creates the
employee, the assignment, the history entry and an audit line reading "Probe
Einstellung"; open positions drop from 13 to 10, and the three that
disappear are exactly the ones with a starter.

Migrations rewrite the affected functions in place rather than restating
them — retyping 165 lines of working PL/pgSQL to change two words is the
larger risk. Each one asserts the result afterwards.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
2026-08-05 12:10:17 +02:00
parent 1271cef879
commit e44f71a60d
7 changed files with 296 additions and 5 deletions

View File

@@ -21,6 +21,15 @@ export async function hireEmployee(payload: {
gender: "m" | "w";
birth_date: string;
sv_nummer?: string;
/**
* Pflicht, weil employees.email NOT NULL ist.
*
* Der Assistent hat die Adresse immer erhoben und in der Zusammenfassung
* angezeigt — sie fehlte nur in dieser Signatur und wurde deshalb
* stillschweigend verworfen. Jede Einstellung scheiterte danach an der
* Spaltenbedingung.
*/
email: string;
phone?: string;
position_id?: string;
team_id?: string;

View File

@@ -58,7 +58,11 @@ export function HireWizard({ open, onClose, openPositions, locations, resumeDraf
isValidSvnr(draft.svNummer, draft.birthDate || null);
const stepValid = [
Boolean(draft.firstName && draft.lastName && draft.birthDate && draft.locationId) && svNummerOk,
// E-Mail gehört zu den Pflichtfeldern, weil die Spalte NOT NULL ist. Ohne
// die Prüfung hier bricht erst die Datenbank ab — am Ende des vierten
// Schritts, nach allen Eingaben.
Boolean(draft.firstName && draft.lastName && draft.birthDate && draft.locationId && draft.email.trim()) &&
svNummerOk,
Boolean(draft.positionId && draft.besetzung),
Boolean(draft.entryDate && draft.workDays.length > 0),
true,
@@ -86,6 +90,7 @@ export function HireWizard({ open, onClose, openPositions, locations, resumeDraf
gender: draft.gender,
birth_date: draft.birthDate,
sv_nummer: draft.svNummer || undefined,
email: draft.email.trim(),
phone: draft.phone || undefined,
position_id: draft.positionId,
location_id: draft.locationId,

View File

@@ -37,7 +37,7 @@ export function StepPerson({ draft, update, locations }: StepPersonProps) {
birthDate={draft.birthDate || null}
/>
<div className="grid grid-cols-1 gap-3 sm:grid-cols-2">
<TextField label="E-Mail (privat)" type="email" value={draft.email} onChange={(email) => update({ email })} />
<TextField label="E-Mail" required type="email" value={draft.email} onChange={(email) => update({ email })} />
<TextField label="Telefon" type="tel" value={draft.phone} onChange={(phone) => update({ phone })} />
</div>
<SelectField

View File

@@ -36,8 +36,23 @@ export async function loadOpenPositions(tx: Tx): Promise<OpenPositionResolved[]>
const [orgMaps, open] = await Promise.all([
loadOrgMaps(tx),
// Unbesetzt heisst: keine am Stichtag laufende Zuordnung. Als NOT EXISTS
// in der Datenbank statt als Filter über alle Planstellen im Speicher.
// Unbesetzt heisst: keine Zuordnung, die noch gilt — **auch keine, die
// erst beginnt.**
//
// Der Unterschied ist kein Feinschliff. Wer unterschrieben hat und am
// 24.09. anfängt, belegt die Planstelle heute schon; sie steht nur noch
// nicht besetzt da. Die frühere Fassung fragte „sitzt heute jemand
// darauf?" und listete solche Stellen als offen — mit „seit 2 Tagen
// unbesetzt" daneben. Aus dieser Liste speist sich auch die Auswahl im
// Einstellungsassistenten, also lud sie dazu ein, dieselbe Stelle ein
// zweites Mal zu besetzen. Aufgefallen wäre das erst am Teilindex der
// Datenbank, nach dem Gespräch mit der zweiten Person.
//
// Eine beendete Zuordnung (valid_to in der Vergangenheit) gibt die Stelle
// dagegen wieder frei — deshalb bleibt die Bedingung auf valid_to.
//
// Als NOT EXISTS in der Datenbank statt als Filter über alle Planstellen
// im Speicher.
tx
.selectFrom("om_positions as p")
.innerJoin("jobs as j", "j.id", "p.job_id")
@@ -51,7 +66,6 @@ export async function loadOpenPositions(tx: Tx): Promise<OpenPositionResolved[]>
.selectFrom("position_assignments as a")
.select("a.id")
.whereRef("a.position_id", "=", "p.id")
.where("a.valid_from", "<=", asOf)
.where((e2) => e2.or([e2("a.valid_to", "is", null), e2("a.valid_to", ">", asOf)]))
)
)

View File

@@ -0,0 +1,84 @@
-- Zwei Funktionen casten auf Typen, die es nicht mehr gibt.
--
-- hire_employee ::weekday[]
-- apply_due_pending_changes ::relationship_type
--
-- Beide Typen waren einmal Aufzählungen und wurden später durch `text` mit
-- einer CHECK-Bedingung ersetzt (chk_work_days_valid). Die Funktionen wurden
-- dabei nicht nachgezogen.
--
-- PL/pgSQL löst Typen in eingebetteten SQL-Anweisungen erst beim **Ausführen**
-- auf. Die Funktionen liessen sich deshalb anlegen und scheitern erst im
-- Betrieb — jede Neueinstellung mit „type weekday[] does not exist", und der
-- nächtliche Lauf, sobald eine fällige Angehörigen-Änderung darin vorkommt.
-- Genau deshalb hat es niemand beim Einspielen gemerkt.
--
-- Ersetzt wird gezielt der Cast an der **aktuellen** Definition, statt beide
-- Funktionen abzuschreiben: 165 Zeilen fremden Code neu zu tippen, um zwei
-- Wörter zu ändern, ist die grössere Fehlerquelle.
create or replace function app_cast_ersetzen(p_funktion text, p_alt text, p_neu text)
returns void
language plpgsql
set search_path = public, pg_temp
as $$
declare
v_def text;
begin
select pg_get_functiondef(p.oid) into v_def
from pg_proc p
join pg_namespace n on n.oid = p.pronamespace
where n.nspname = 'public' and p.proname = p_funktion
limit 1;
if v_def is null then
raise exception 'Funktion %() nicht gefunden.', p_funktion;
end if;
if position(p_alt in v_def) = 0 then
raise notice '%(): % kommt nicht (mehr) vor — nichts zu tun.', p_funktion, p_alt;
return;
end if;
execute replace(v_def, p_alt, p_neu);
raise notice '%(): % -> %', p_funktion, p_alt, p_neu;
end;
$$;
select app_cast_ersetzen('hire_employee', '::weekday[]', '::text[]');
select app_cast_ersetzen('apply_due_pending_changes', '::relationship_type', '::text');
-- Das Werkzeug wird nicht aufbewahrt: eine Funktion, die beliebigen Text in
-- eine Funktionsdefinition schreibt und ausführt, soll nicht dauerhaft im
-- Schema stehen.
drop function app_cast_ersetzen(text, text, text);
-- ═══ Gegenprobe ══════════════════════════════════════════════════
-- Keine Funktion im Schema darf mehr auf einen Typ casten, den es nicht
-- gibt. Das prüft nicht nur die zwei bekannten Stellen, sondern schliesst
-- aus, dass beim Ersetzen eine dritte übersehen wurde.
do $$
declare
r record;
v_treffer text;
begin
for r in
select p.proname, pg_get_functiondef(p.oid) as def
from pg_proc p
join pg_namespace n on n.oid = p.pronamespace
where n.nspname = 'public' and p.prokind = 'f'
loop
for v_treffer in
select m[1] from regexp_matches(r.def, '::\s*([a-z_][a-z0-9_]*)\s*(?:\[\])?', 'gi') m
loop
if not exists (select 1 from pg_type where typname = lower(v_treffer))
and lower(v_treffer) not in (
'text', 'int', 'integer', 'boolean', 'bool', 'date', 'uuid', 'jsonb', 'json',
'numeric', 'timestamptz', 'varchar', 'bigint', 'smallint', 'real', 'interval', 'time'
) then
raise exception 'Funktion %() castet auf unbekannten Typ „%".', r.proname, v_treffer;
end if;
end loop;
end loop;
end;
$$;

View File

@@ -0,0 +1,113 @@
-- auth.uid() aus den Geschäftsfunktionen entfernen.
--
-- Die Anwendung verbindet sich als eigene Rolle ohne BYPASSRLS. Diese Rolle
-- hat kein Recht auf das Schema `auth` — und jede Funktion, die dort etwas
-- aufruft, scheitert mit „permission denied for schema auth".
--
-- Das trifft **jede schreibende Aktion**: Einstellen, Versetzen, Befördern,
-- Austritt, Notizen, Planstellen. Aufgefallen ist es beim Nachstellen einer
-- Neueinstellung; zuvor lief alles über eine Rolle, die das Schema sehen
-- durfte.
--
-- app_current_user_id() liefert dasselbe und funktioniert auf jedem
-- PostgreSQL — es liest den transaktionslokalen Sitzungskontext, den lib/db
-- setzt.
create or replace function app_uid_ersetzen(p_funktion text)
returns void
language plpgsql
set search_path = public, pg_temp
as $$
declare
v_def text;
begin
select pg_get_functiondef(p.oid) into v_def
from pg_proc p
join pg_namespace n on n.oid = p.pronamespace
where n.nspname = 'public' and p.proname = p_funktion
limit 1;
if v_def is null then
raise exception 'Funktion %() nicht gefunden.', p_funktion;
end if;
if position('auth.uid()' in v_def) = 0 then
raise notice '%(): bereits umgestellt.', p_funktion;
return;
end if;
execute replace(v_def, 'auth.uid()', 'app_current_user_id()');
raise notice '%(): umgestellt.', p_funktion;
end;
$$;
select app_uid_ersetzen('add_employee_dependent');
select app_uid_ersetzen('add_employee_note');
select app_uid_ersetzen('adjust_karenz_return');
select app_uid_ersetzen('complete_employee_note');
select app_uid_ersetzen('create_position');
select app_uid_ersetzen('delete_employee_dependent');
select app_uid_ersetzen('delete_position');
select app_uid_ersetzen('hire_employee');
select app_uid_ersetzen('promote_employee');
select app_uid_ersetzen('record_karenz_return');
select app_uid_ersetzen('rehire_employee');
select app_uid_ersetzen('start_karenz');
select app_uid_ersetzen('terminate_employee');
select app_uid_ersetzen('transfer_employee');
drop function app_uid_ersetzen(text);
-- ═══ Den Rückfall selbst absichern ═══════════════════════════════
-- app_current_user_id() behält seinen Übergangszweig auf auth.uid(), fängt
-- aber bisher nur „Funktion fehlt". Für eine Rolle ohne Recht auf das Schema
-- kommt stattdessen insufficient_privilege — und die Funktion warf, statt
-- null zu liefern. Das fiel nicht auf, weil der Zweig nur ohne
-- Sitzungskontext erreicht wird; genau dann soll aber „niemand angemeldet"
-- herauskommen und kein Fehler.
create or replace function app_current_user_id()
returns uuid
language plpgsql
stable
security definer
set search_path = public, pg_temp
as $$
declare
v_id uuid;
begin
v_id := nullif(current_setting('app.user_id', true), '')::uuid;
if v_id is not null then
return v_id;
end if;
begin
execute 'select auth.uid()' into v_id;
exception
when undefined_function or invalid_schema_name or undefined_table or insufficient_privilege then
v_id := null;
end;
return v_id;
end;
$$;
-- ═══ Gegenprobe ══════════════════════════════════════════════════
do $$
declare r record;
begin
for r in
select p.proname
from pg_proc p
join pg_namespace n on n.oid = p.pronamespace
where n.nspname = 'public' and p.prokind = 'f'
and p.proname <> 'app_current_user_id'
and pg_get_functiondef(p.oid) like '%auth.uid()%'
loop
raise exception 'Funktion %() ruft weiterhin auth.uid() auf.', r.proname;
end loop;
-- Ohne Kontext muss die Kennung null sein und darf nicht werfen.
perform set_config('app.user_id', '', true);
if app_current_user_id() is not null then
raise exception 'app_current_user_id() liefert ohne Kontext eine Kennung.';
end if;
end;
$$;

View File

@@ -0,0 +1,66 @@
-- Klammern in hire_employee.
--
-- Der Protokolleintrag baute den Namen so:
--
-- payload->>'first_name' || ' ' || payload->>'last_name'
--
-- In PostgreSQL bindet `||` **stärker** als `->>`. Gelesen wird also
--
-- payload ->> ('first_name' || ' ' || payload) ->> 'last_name'
--
-- und das endet in „operator does not exist: text ->> unknown". Jede
-- Neueinstellung scheiterte daran.
--
-- Warum es niemandem auffiel: davor stand in derselben Anweisung ein Aufruf
-- von auth.uid(). Die Rechteprüfung auf das Schema `auth` schlug schon
-- während der Analyse fehl, und der Parser kam nie bis zu diesem Ausdruck.
-- Ein Fehler hat den anderen verdeckt — beide mussten weg, damit eine
-- Einstellung durchläuft.
do $$
declare
v_def text;
v_alt constant text := 'payload->>''first_name'' || '' '' || payload->>''last_name''';
v_neu constant text := '(payload->>''first_name'') || '' '' || (payload->>''last_name'')';
begin
select pg_get_functiondef(p.oid) into v_def
from pg_proc p
join pg_namespace n on n.oid = p.pronamespace
where n.nspname = 'public' and p.proname = 'hire_employee'
limit 1;
if v_def is null then
raise exception 'hire_employee() nicht gefunden.';
end if;
if position(v_alt in v_def) = 0 then
raise notice 'hire_employee(): bereits geklammert.';
else
execute replace(v_def, v_alt, v_neu);
raise notice 'hire_employee(): geklammert.';
end if;
end;
$$;
-- ═══ Gegenprobe ══════════════════════════════════════════════════
-- Der Ausdruck selbst, in beiden Lesarten. Ohne Klammern wirft er; mit
-- Klammern kommt der Name heraus. Das hält die Regel fest, damit sie beim
-- nächsten Mal nicht neu entdeckt werden muss.
do $$
declare
v_payload jsonb := '{"first_name": "Anna", "last_name": "Berger"}'::jsonb;
v_name text;
begin
v_name := (v_payload->>'first_name') || ' ' || (v_payload->>'last_name');
if v_name <> 'Anna Berger' then
raise exception 'Geklammert ergibt „%" statt „Anna Berger".', v_name;
end if;
begin
execute $probe$ select ('{"a":"x"}'::jsonb)->>'a' || ' ' || ('{"a":"x"}'::jsonb)->>'a' $probe$;
raise exception 'Ungeklammert wirft nicht mehr — die Vorrangregel hat sich geändert, die Prüfung ist wertlos geworden.';
exception
when undefined_function then null; -- erwartet: text ->> unknown
end;
end;
$$;