Janady13/missionmatch / db/migrations/0056_activity_integrity.sql

Source identity

Field	Recorded value
Source type	GitHub source
Original record	Janady13/missionmatch / db/migrations/0056_activity_integrity.sql
Date	Not specified in source
Repository	Janady13/missionmatch
Revision	720d47fae476b0d38477ae4401697d372b1ea2bc
Branch	feature/opportunity-to-outcome
Path or archive member	db/migrations/0056_activity_integrity.sql
Original SHA-256	c321a9468827066d04f94d23497350cc49d8b466cebd6184805d3f264e06be87
Git blob SHA	13592be534360c5cc1105785d94943147f84e02d
Reading-copy SHA-256	c321a9468827066d04f94d23497350cc49d8b466cebd6184805d3f264e06be87

Original source ↗

Every recorded occurrence

These 2 records contain identical original source bytes. Their paths and revisions remain part of the archive.

Original record	Branch or archive	Revision / identifier	Original location
Janady13/missionmatch / db/migrations/0056_activity_integrity.sql	feature/opportunity-to-outcome	720d47fae476b0d38477ae4401697d372b1ea2bc	Open original
Janady13/missionmatch / db/migrations/0056_activity_integrity.sql	feature/aioe-upgrade	2178e7298d0e5d9af83afa12402fd587f0ba6ed8	Open original

Recovered source transcript

Download complete source text ↓171 lines · 9.6 KB

-- +goose Up
-- Makes the recruiter activity log a record rather than a scratchpad. Four defects in 0049, each
-- reproduced against a live database before this was written.
--
-- A. An activity could be dated in the future. A recruiter could pre log a month of calls, and today's
--    productivity number would count work that has not happened.
--
-- B. A human could set auto_captured = true on their own row. That column exists for exactly one reason,
--    to keep captured effort and self reported effort separable so a productivity comparison means
--    something. A flag the subject can set about themselves is not a flag.
--
-- C. A logged activity could be rewritten or deleted. Verified: kind changed from call to meeting,
--    occurred_at moved five days back, then the row deleted outright. A record that can be edited after
--    the fact cannot support the metrics Phase 17 derives from it.
--
-- D. Deleting a recruiter deleted their entire activity history, because actor_user_id cascaded.
--    Offboarding someone erased the record of the work they did, and it disagreed with
--    application_stage_events, which nulls the actor and keeps the row.

-- ---------------------------------------------------------------------------
-- D. The history outlives the person.
--
-- actor_name is denormalised for the same reason it was on the two event tables: once actor_user_id is
-- nulled, a joined name would rewrite a departed recruiter's whole history into "unknown". What the
-- record should say is who did the work THEN.
--
-- This is offboarding, not erasure. Removing a person's name from the record is a separate, deliberate
-- operation and should not be a side effect of closing an account.
ALTER TABLE recruiter_activities ADD COLUMN actor_name text NOT NULL DEFAULT '';

UPDATE recruiter_activities a
   SET actor_name = COALESCE((SELECT u.full_name FROM users u
                               WHERE u.organization_id = a.organization_id AND u.id = a.actor_user_id), '');

ALTER TABLE recruiter_activities ALTER COLUMN actor_user_id DROP NOT NULL;
ALTER TABLE recruiter_activities DROP CONSTRAINT recruiter_activities_organization_id_actor_user_id_fkey;
-- The column list is required. A bare SET NULL on a composite key nulls organization_id too, which is
-- NOT NULL, so deleting a user would abort. Migration 0053 repaired twenty three constraints that made
-- that mistake.
ALTER TABLE recruiter_activities ADD CONSTRAINT recruiter_activities_actor_fkey
    FOREIGN KEY (organization_id, actor_user_id) REFERENCES users (organization_id, id)
    ON DELETE SET NULL (actor_user_id);

-- ---------------------------------------------------------------------------
-- A, B and C, all enforced at insert and update time.
--
-- auto_captured is SET by the trigger from a transaction local flag, never read from the inserted row.
-- That is what makes it trustworthy: the value the caller supplied is discarded, so no request body and
-- no direct INSERT can claim it. The service sets the flag only on genuine capture paths, and the HTTP
-- input type has no such field at all, so the API cannot express the claim.
-- +goose StatementBegin
CREATE OR REPLACE FUNCTION rm_activity_guard() RETURNS trigger AS $$
BEGIN
    IF TG_OP = 'INSERT' THEN
        -- A. Future dating. A small grace absorbs clock skew between the application and the database
        -- without allowing a date anyone would choose deliberately.
        IF NEW.occurred_at > now() + interval '5 minutes' THEN
            RAISE EXCEPTION 'an activity cannot be logged in the future'
                USING ERRCODE = '22007';
        END IF;
        -- A loose absolute floor, only to catch a mistyped year. It is deliberately NOT the organization's
        -- creation date: a client onboarding this week must be able to log the calls they made last week,
        -- and tying the floor to the account would refuse every one of them during the first days.
        IF NEW.occurred_at < timestamptz '2000-01-01' THEN
            RAISE EXCEPTION 'that date looks mistyped'
                USING ERRCODE = '22007';
        END IF;
        -- B. The caller's claim is discarded and replaced by the transaction local flag.
        -- COALESCE is load bearing. current_setting with missing_ok returns NULL when the flag is unset,
        -- and NULL = '1' is NULL rather than false, so a bare comparison writes NULL into a NOT NULL
        -- column and every human logged activity fails to insert.
        NEW.auto_captured := (COALESCE(current_setting('app.rm_system_activity', true), '') = '1');
        RETURN NEW;
    END IF;

    -- C. Only the body may change. Everything a metric is computed from is fixed at insert, so a logged
    -- call cannot become a meeting, move in time, change hands, or start claiming it was captured.
    IF NEW.actor_user_id  IS DISTINCT FROM OLD.actor_user_id
       OR NEW.kind        IS DISTINCT FROM OLD.kind
       OR NEW.occurred_at IS DISTINCT FROM OLD.occurred_at
       OR NEW.auto_captured IS DISTINCT FROM OLD.auto_captured
       OR NEW.candidate_id   IS DISTINCT FROM OLD.candidate_id
       OR NEW.contact_id     IS DISTINCT FROM OLD.contact_id
       OR NEW.requisition_id IS DISTINCT FROM OLD.requisition_id
       OR NEW.application_id IS DISTINCT FROM OLD.application_id THEN
        -- The one exception is the referential null out when a user is deleted, which changes
        -- actor_user_id from a value to NULL and nothing else. Same narrow carve out as the event tables.
        IF NOT (OLD.actor_user_id IS NOT NULL AND NEW.actor_user_id IS NULL
                AND (to_jsonb(NEW) - 'actor_user_id') = (to_jsonb(OLD) - 'actor_user_id')) THEN
            RAISE EXCEPTION
                'a logged activity is a record: only its note may be corrected'
                USING ERRCODE = '42501';
        END IF;
    END IF;
    RETURN NEW;
END;
$$ LANGUAGE plpgsql;
-- +goose StatementEnd

CREATE TRIGGER recruiter_activities_guard
    BEFORE INSERT OR UPDATE ON recruiter_activities
    FOR EACH ROW EXECUTE FUNCTION rm_activity_guard();

-- C, the second barrier. Deleting is not a correction, it is a hole in the record.
REVOKE DELETE ON recruiter_activities FROM missionmatch_app;

-- +goose StatementBegin
CREATE OR REPLACE FUNCTION rm_forbid_activity_delete() RETURNS trigger AS $$
BEGIN
    IF current_user = 'missionmatch_migrator' THEN
        RETURN OLD; -- lifecycle and cascades, the same exemption as migration 0050
    END IF;
    RAISE EXCEPTION 'activity history cannot be deleted' USING ERRCODE = '42501';
END;
$$ LANGUAGE plpgsql;
-- +goose StatementEnd

CREATE TRIGGER recruiter_activities_no_delete
    BEFORE DELETE ON recruiter_activities
    FOR EACH ROW EXECUTE FUNCTION rm_forbid_activity_delete();

-- The daily productivity read: one recruiter's logged effort in a window, excluding captured rows.
-- Leading with the actor and the day is what makes a per person per day rollup an index scan.
CREATE INDEX idx_activities_actor_day
    ON recruiter_activities (organization_id, actor_user_id, occurred_at DESC)
    WHERE NOT auto_captured;

-- +goose Down
DROP INDEX IF EXISTS idx_activities_actor_day;
DROP TRIGGER IF EXISTS recruiter_activities_no_delete ON recruiter_activities;
DROP FUNCTION IF EXISTS rm_forbid_activity_delete();
DROP TRIGGER IF EXISTS recruiter_activities_guard ON recruiter_activities;
DROP FUNCTION IF EXISTS rm_activity_guard();
GRANT DELETE ON recruiter_activities TO missionmatch_app;
ALTER TABLE recruiter_activities DROP CONSTRAINT IF EXISTS recruiter_activities_actor_fkey;
-- +goose StatementBegin
-- REFUSE rather than reattribute or delete. The previous body of this down did both, and each half was
-- worse than the bug it was working around.
--
-- It picked a replacement actor with SELECT u.id FROM users u WHERE u.organization_id = ... LIMIT 1, with
-- no ORDER BY. LIMIT without ORDER BY has no defined answer: it returns whichever row the plan happens to
-- reach first, so the same rollback run twice can name two different people, and the person it names had
-- nothing to do with the work. That is not a data fix, it is a forged signature on somebody else's record.
-- Then it deleted whatever was left, which erases the activity outright.
--
-- Defect D above is precisely that offboarding someone erased the record of the work they did. A NULL
-- actor is the up's intended, documented state: the deed is kept, the actor reference is released, and
-- actor_name still says who did it at the time. A down migration may drop what its up added; it may not
-- rewrite authorship the application recorded, and it may not delete history to make a constraint fit.
--
-- The operator decides what those rows should become, and naming the count is what makes that decision
-- possible. actor_name on each row still holds the departed person's name, so the reassignment can be done
-- knowingly, one desk at a time, before this rollback is retried.
DO $$
DECLARE n bigint;
BEGIN
    SELECT count(*) INTO n FROM recruiter_activities WHERE actor_user_id IS NULL;
    IF n > 0 THEN
        RAISE EXCEPTION
            'refusing to roll back: % activity row(s) have no actor because that user was deleted. '
            'Rolling back would reattribute them to an arbitrary surviving user and delete the rest. '
            'Reassign them deliberately using actor_name, or export and remove them, then retry.', n;
    END IF;
END
$$;
-- +goose StatementEnd
ALTER TABLE recruiter_activities ALTER COLUMN actor_user_id SET NOT NULL;
ALTER TABLE recruiter_activities ADD CONSTRAINT recruiter_activities_organization_id_actor_user_id_fkey
    FOREIGN KEY (organization_id, actor_user_id) REFERENCES users (organization_id, id) ON DELETE CASCADE;
ALTER TABLE recruiter_activities DROP COLUMN IF EXISTS actor_name;