Janady13/missionmatch / db/migrations/0052_requisition_hold_clock.sql

1-- +goose Up 2-- Corrects the hold clock on requisitions and adds the status event trail. 3-- 4-- 0047 stored held_days as an int. That silently discards every hold shorter than a day. A job order 5-- put on hold Friday evening and resumed Monday morning three separate times accrued 30 real hours of 6-- hold and would have recorded zero, because each period truncates to 0 before it is added. The error is 7-- one directional, it always under counts, so adjusted time to fill would always read longer than the 8-- truth and no amount of data would reveal it. 9-- 10-- The accumulator is now exact seconds as numeric. Seconds rather than an interval on purpose: summing 11-- intervals that carry a day component and then subtracting them from a timestamptz is ambiguous across 12-- a daylight saving boundary, because a calendar day is not always 86400 seconds. EXTRACT(EPOCH FROM 13-- (a - b)) is an unambiguous count of elapsed seconds at microsecond resolution, and every arithmetic 14-- step after that is exact numeric, never floating point. 15-- 16-- held_since is the open bracket of the current hold. The pairing invariant below is what makes the 17-- accumulator trustworthy: it is impossible to be on hold without an open bracket, and impossible to 18-- carry an open bracket while not on hold. Without it a requisition closed while on hold would keep 19-- accruing hold time forever against a dead row. 20-- 21-- Safe to rewrite the column: requisitions is empty in every environment at the time of this migration. 22 23ALTER TABLE requisitions DROP COLUMN held_days; 24 25ALTER TABLE requisitions 26 ADD COLUMN held_seconds numeric(20,6) NOT NULL DEFAULT 0 CHECK (held_seconds >= 0), 27 ADD COLUMN held_since timestamptz; 28 29-- The pairing invariant. Together with requisitions_closed_at_chk from 0047 this makes the three clocks 30-- on a requisition, opened_at, closed_at and the hold accumulator, mutually consistent for every legal 31-- status, enforced by the database rather than by the care of whoever writes the next service method. 32ALTER TABLE requisitions ADD CONSTRAINT requisitions_held_since_chk CHECK ( 33 (status = 'on_hold' AND held_since IS NOT NULL) 34 OR (status <> 'on_hold' AND held_since IS NULL)); 35 36-- A hold cannot have started before the order was opened, and cannot be settled against a close that 37-- happened first. Both would make the adjusted age negative, which is the one arithmetic outcome that 38-- is never a real measurement. 39ALTER TABLE requisitions ADD CONSTRAINT requisitions_held_since_after_open_chk CHECK ( 40 held_since IS NULL OR held_since >= opened_at); 41 42-- --------------------------------------------------------------------------- 43-- Closes an integrity hole in 0047. hiring_manager_id referenced client_contacts by (organization_id, 44-- id), which proves the contact exists somewhere in the tenant but not that it belongs to THIS client. 45-- Nothing stopped a job order at one client naming a hiring manager who works at another, and the first 46-- symptom would have been a submittal emailed to the wrong company. 47-- 48-- The fix is a composite foreign key that carries client_id, so the database checks the contact belongs 49-- to the client on the order. It needs a unique key on the referenced side to point at. 50ALTER TABLE client_contacts ADD CONSTRAINT uq_client_contacts_client_id UNIQUE (organization_id, client_id, id); 51 52ALTER TABLE requisitions DROP CONSTRAINT requisitions_organization_id_hiring_manager_id_fkey; 53 54-- A composite foreign key with any NULL column is not checked at all under MATCH SIMPLE, so a manager 55-- attached to a clientless order is not verified by the database. The obvious remedy, a CHECK requiring a 56-- client whenever a manager is named, was tried and removed, because it cannot coexist with the cascade: 57-- 58-- Deleting a client fires two independent referential actions on this table, one nulling client_id and 59-- one nulling hiring_manager_id by way of the cascade through client_contacts. Postgres applies each as 60-- its own statement and evaluates row constraints after each, so whichever runs first leaves the row 61-- momentarily inconsistent and the CHECK aborts the delete. Postgres does not support DEFERRABLE on a 62-- CHECK, and MATCH FULL is not usable here because organization_id is never null, so it would reject 63-- every legitimately clientless order. Verified against the live schema: the constraint reintroduced 64-- the exact deletion failure that migration 0053 exists to remove. 65-- 66-- The rule is therefore enforced in the service, which rejects a hiring manager without a client before 67-- the insert is attempted. The database still guarantees the part that matters and that the application 68-- cannot check without a race: when both are present, the manager works at that client. 69-- The column list on SET NULL is load bearing, not decoration. A bare ON DELETE SET NULL on a composite 70-- key nulls EVERY referencing column, so deleting one contact would blank organization_id, which is NOT 71-- NULL and would abort, and would also silently detach the order from its client. Naming the column 72-- confines the effect to the manager, which is the only part that stopped being true. Postgres 15 and 73-- later support this form and this deployment is on 16. 74ALTER TABLE requisitions ADD CONSTRAINT requisitions_hiring_manager_fkey 75 FOREIGN KEY (organization_id, client_id, hiring_manager_id) 76 REFERENCES client_contacts (organization_id, client_id, id) 77 ON DELETE SET NULL (hiring_manager_id); 78 79-- Deleting a client leaves hiring_manager_id dangling, and this is what clears it. 80-- 81-- Verified behaviour: deleting a client fires two referential actions on this table. The one nulling 82-- client_id runs first, and once client_id is NULL the composite key contains a NULL, so under MATCH 83-- SIMPLE the manager key is no longer enforced and the cascade that deletes the client's contacts never 84-- nulls hiring_manager_id. The row is left pointing at a contact that no longer exists. 85-- 86-- The effect is invisible in the interface, because the name is resolved by a join that now finds 87-- nothing, which is precisely why it needs handling here rather than being left to be noticed. A trigger 88-- is used because no declarative constraint can express it: the rule spans two foreign keys whose actions 89-- Postgres applies as separate statements. 90-- 91-- Restricted to UPDATE deliberately. On INSERT the same situation is a mistake by the caller, and the 92-- service rejects it with a message naming the field rather than silently discarding what was typed. 93-- +goose StatementBegin 94CREATE OR REPLACE FUNCTION rm_clear_orphaned_hiring_manager() RETURNS trigger AS $$ 95BEGIN 96 IF NEW.client_id IS NULL AND NEW.hiring_manager_id IS NOT NULL THEN 97 NEW.hiring_manager_id := NULL; 98 END IF; 99 RETURN NEW; 100END; 101$$ LANGUAGE plpgsql; 102-- +goose StatementEnd 103 104CREATE TRIGGER requisitions_clear_orphaned_manager 105 BEFORE UPDATE ON requisitions 106 FOR EACH ROW EXECUTE FUNCTION rm_clear_orphaned_hiring_manager(); 107 108-- --------------------------------------------------------------------------- 109-- requisition_events. Append only, because every metric Phase 17 reports about how long an order sat in 110-- a state is derived from this table, and a derived metric is only auditable if its inputs cannot be 111-- rewritten after the fact. Recording it from the first status change means the history exists when the 112-- reporting is built, rather than starting from whenever someone remembered to add it. 113-- --------------------------------------------------------------------------- 114CREATE TABLE requisition_events ( 115 organization_id uuid NOT NULL REFERENCES organizations(id), 116 id uuid NOT NULL DEFAULT gen_random_uuid(), 117 requisition_id uuid NOT NULL, 118 from_status text, 119 to_status text NOT NULL, 120 -- Seconds spent in from_status, closed out at the moment of this event. Stored rather than derived 121 -- so a report does not have to self join the table to the previous row per requisition, and so the 122 -- number stays correct if a future migration ever changes what a status means. 123 seconds_in_from numeric(20,6) CHECK (seconds_in_from IS NULL OR seconds_in_from >= 0), 124 note text NOT NULL DEFAULT '', 125 actor_user_id uuid, 126 -- The actor's name at the time of the event, copied rather than joined. 127 -- 128 -- actor_user_id is nulled when a user is deleted, which is required for offboarding and erasure. If 129 -- the name were only reachable through that key, deleting a recruiter would silently rewrite years of 130 -- history into "unknown" and destroy the audit value of the table this trigger exists to protect. 131 -- Denormalising the name keeps the history legible after the reference is gone, and it is the correct 132 -- value regardless: it records who acted THEN, not who happens to hold that user row now. 133 actor_name text NOT NULL DEFAULT '', 134 occurred_at timestamptz NOT NULL DEFAULT now(), 135 PRIMARY KEY (organization_id, id), 136 FOREIGN KEY (organization_id, requisition_id) REFERENCES requisitions (organization_id, id) ON DELETE CASCADE, 137 -- The column list is required. A bare SET NULL on a composite key nulls organization_id too, which is 138 -- NOT NULL, so deleting any user would abort. Migration 0053 repairs twenty three constraints across 139 -- this schema that made exactly that mistake; this one is written correctly from the start. 140 FOREIGN KEY (organization_id, actor_user_id) REFERENCES users (organization_id, id) 141 ON DELETE SET NULL (actor_user_id), 142 -- The first event of a requisition's life has no predecessor. Every later one must name what it left, 143 -- so the chain cannot silently lose a link. 144 CONSTRAINT requisition_events_from_chk CHECK ( 145 (from_status IS NULL AND seconds_in_from IS NULL) 146 OR (from_status IS NOT NULL AND seconds_in_from IS NOT NULL)), 147 CONSTRAINT requisition_events_to_chk CHECK ( 148 to_status IN ('draft', 'open', 'on_hold', 'filled', 'closed', 'cancelled')), 149 CONSTRAINT requisition_events_from_vocab_chk CHECK ( 150 from_status IS NULL OR from_status IN ('draft', 'open', 'on_hold', 'filled', 'closed', 'cancelled')) 151); 152 153CREATE INDEX idx_requisition_events_req ON requisition_events (organization_id, requisition_id, occurred_at); 154 155ALTER TABLE requisition_events ENABLE ROW LEVEL SECURITY; 156ALTER TABLE requisition_events FORCE ROW LEVEL SECURITY; 157CREATE POLICY requisition_events_isolation ON requisition_events 158 USING (organization_id = NULLIF(current_setting('app.current_org_id', true), '')::uuid) 159 WITH CHECK (organization_id = NULLIF(current_setting('app.current_org_id', true), '')::uuid); 160 161-- Only SELECT and INSERT reach the application role. The append only property has two independent 162-- barriers, the missing grant and the trigger, so neither one being wrong on its own is enough to let 163-- history be rewritten. 164GRANT SELECT, INSERT ON requisition_events TO missionmatch_app; 165 166-- --------------------------------------------------------------------------- 167-- Widens the append only guard by exactly one case, and fixes the same latent conflict on 168-- application_stage_events, which shares this function. 169-- 170-- A foreign key declared ON DELETE SET NULL performs an UPDATE on the referencing row. The guard refuses 171-- every UPDATE from the application role, so deleting a user who ever changed a requisition status or 172-- moved a candidate through a stage would be refused by the trigger even after 0053 makes the constraint 173-- itself valid. That is the same shape of defect as 0050: a rule written to protect history instead 174-- making a legitimate lifecycle operation impossible. 175-- 176-- The exemption is deliberately narrow. It permits only an update that clears actor_user_id from a value 177-- to NULL while every other column is byte identical, which is precisely and only what the referential 178-- action does. The jsonb comparison is what makes that precise rather than approximate: any other column 179-- differing makes the two objects unequal and the update is refused. History still cannot be rewritten, 180-- and the actor's name survives in actor_name regardless. 181-- +goose StatementBegin 182CREATE OR REPLACE FUNCTION rm_forbid_event_mutation() RETURNS trigger AS $$ 183BEGIN 184 IF current_user = 'missionmatch_migrator' THEN 185 IF TG_OP = 'DELETE' THEN 186 RETURN OLD; 187 END IF; 188 RETURN NEW; 189 END IF; 190 IF TG_OP = 'UPDATE' 191 AND OLD.actor_user_id IS NOT NULL 192 AND NEW.actor_user_id IS NULL 193 AND (to_jsonb(NEW) - 'actor_user_id') = (to_jsonb(OLD) - 'actor_user_id') THEN 194 RETURN NEW; 195 END IF; 196 RAISE EXCEPTION 'append-only table: % not permitted for role %', TG_OP, current_user; 197END; 198$$ LANGUAGE plpgsql; 199-- +goose StatementEnd 200 201CREATE TRIGGER requisition_events_append_only 202 BEFORE UPDATE OR DELETE ON requisition_events 203 FOR EACH ROW EXECUTE FUNCTION rm_forbid_event_mutation(); 204CREATE TRIGGER requisition_events_no_truncate 205 BEFORE TRUNCATE ON requisition_events 206 FOR EACH STATEMENT EXECUTE FUNCTION rm_forbid_event_mutation(); 207 208-- Listing a client's job orders and a recruiter's own desk are the two reads the dashboard makes on 209-- every load, so both get an index that already carries the filter. 210CREATE INDEX idx_requisitions_org_client ON requisitions (organization_id, client_id, status) WHERE deleted_at IS NULL; 211CREATE INDEX idx_requisitions_org_owner ON requisitions (organization_id, owner_user_id, status) WHERE deleted_at IS NULL; 212 213-- +goose Down 214-- Restores the 0050 form of the guard, without the referential exemption. 215-- +goose StatementBegin 216CREATE OR REPLACE FUNCTION rm_forbid_event_mutation() RETURNS trigger AS $$ 217BEGIN 218 IF current_user = 'missionmatch_migrator' THEN 219 IF TG_OP = 'DELETE' THEN 220 RETURN OLD; 221 END IF; 222 RETURN NEW; 223 END IF; 224 RAISE EXCEPTION 'append-only table: % not permitted for role %', TG_OP, current_user; 225END; 226$$ LANGUAGE plpgsql; 227-- +goose StatementEnd 228ALTER TABLE requisitions DROP CONSTRAINT IF EXISTS requisitions_hiring_manager_fkey; 229ALTER TABLE client_contacts DROP CONSTRAINT IF EXISTS uq_client_contacts_client_id; 230-- The column list form is kept on the way back down, deliberately, and this is the one place this down 231-- does not restore the previous definition byte for byte. The original was a bare ON DELETE SET NULL on a 232-- composite key, which nulls EVERY column in the key including organization_id, and organization_id is 233-- NOT NULL on requisitions, so deleting a client contact aborts and the contact becomes undeletable. 234-- Migration 0053 exists to repair exactly that defect across twenty three constraints. Reinstating it here 235-- would hand 0053 back a broken constraint it had already fixed and silently break contact offboarding for 236-- anyone who rolled this migration back. A down migration restores the previous SHAPE of the schema, not a 237-- known defect that a later migration has already ruled out. 238ALTER TABLE requisitions ADD CONSTRAINT requisitions_organization_id_hiring_manager_id_fkey 239 FOREIGN KEY (organization_id, hiring_manager_id) 240 REFERENCES client_contacts (organization_id, id) ON DELETE SET NULL (hiring_manager_id); 241DROP TRIGGER IF EXISTS requisitions_clear_orphaned_manager ON requisitions; 242DROP FUNCTION IF EXISTS rm_clear_orphaned_hiring_manager(); 243DROP INDEX IF EXISTS idx_requisitions_org_owner; 244DROP INDEX IF EXISTS idx_requisitions_org_client; 245DROP TABLE IF EXISTS requisition_events; 246ALTER TABLE requisitions DROP CONSTRAINT IF EXISTS requisitions_held_since_after_open_chk; 247ALTER TABLE requisitions DROP CONSTRAINT IF EXISTS requisitions_held_since_chk; 248ALTER TABLE requisitions DROP COLUMN IF EXISTS held_since; 249ALTER TABLE requisitions DROP COLUMN IF EXISTS held_seconds; 250ALTER TABLE requisitions ADD COLUMN held_days int NOT NULL DEFAULT 0 CHECK (held_days >= 0); 251