# T5 — Foreign Key Restoration Audit (READ-ONLY)

**Date:** 2026-07-14
**Status:** Audit only. **No constraints added. No data modified. No migrations run.**
**Follows:** `STORAGE_ENGINE_TRANSACTION_AUDIT.md` (complete), `PAYROLL_TRANSACTION_DEFECTS.md` (complete)

---

## Executive summary

**51 foreign keys declared in the migrations do not exist in production.**

MyISAM parses `FOREIGN KEY` definitions and **silently discards them**. Every table created while the server default was MyISAM therefore lost its constraints at CREATE time. Converting the engine to InnoDB (Batches 1–4) made those tables *capable* of enforcing FKs — it did **not** restore the constraints themselves.

| Schema | Foreign keys |
|---|---|
| `stepup_admin_db` (production, converted in place) | **69** |
| `stepup_test_db` (rebuilt from migrations, canonical) | **120** |
| **Missing in production** | **51** |
| Extra in production (not in migrations) | **0** |

Production's FK set is a strict *subset* of the canonical set — nothing has drifted the other way, which makes this a pure restoration problem rather than a reconciliation one.

**Data readiness is good.** Across all 51 missing FKs:

| Check | Result |
|---|---|
| Column type / signedness mismatches | **0** |
| Missing columns | **0** |
| Orphan rows | **62**, across **3** constraints |
| Everything else | clean and ready to constrain |

Only three constraints are blocked, and all three are legacy garbage that MyISAM tolerated — not live corruption.

---

## 1. Method

1. Enumerated FKs in both schemas from `information_schema.KEY_COLUMN_USAGE` joined to `REFERENTIAL_CONSTRAINTS` (which carries the real `DELETE_RULE` / `UPDATE_RULE`).
2. Treated `stepup_test_db` as canonical: it was rebuilt with `migrate:fresh`, so its FK set is exactly what the migrations *intend*.
3. Diffed, then for every missing FK checked against **production data**: child/parent column types, nullability, orphan rows (`LEFT JOIN ... WHERE parent IS NULL`, ignoring NULLs), and whether a leading index exists.

---

## 2. Blockers — the 62 orphan rows

### B1/B2 — `invoices.studentId` and `payments.studentId` → `students.id` (1 orphan each)

```
invoices.id=2  references missing students.id=2
payments.id=2  references missing students.id=2
```

`students` holds 34 rows, max id 68, and **zero** soft-deleted rows. Student id 2 was hard-deleted at some point — leaving **an invoice and a payment attached to a student who does not exist**. These are financial records pointing at nobody. MyISAM allowed it.

**Decision required.** Options, in order of preference:
1. Re-point both rows at a "Deleted student" placeholder record (preserves the money trail).
2. Make `studentId` nullable and set it NULL (loses attribution).
3. Delete the invoice and payment (destroys financial history — **not recommended** without accounting sign-off).

This is a data-remediation decision, not a technical one. It must be made before B1/B2 can be constrained.

### B3 — `role_permissions.roleId` → `user_roles.id` (60 orphans)

```
roleId=5 -> 22 role_permissions rows   (no such user_role)
roleId=6 -> 19 role_permissions rows   (no such user_role)
roleId=7 -> 19 role_permissions rows   (no such user_role)
existing user_roles ids: 1, 2, 3, 4   (4 rows, 0 soft-deleted)
users still on roles 5/6/7: 0, 0, 0
```

Roles 5, 6 and 7 were **hard-deleted** (they are not soft-deleted — the rows are simply gone), and their 60 permission grants were left behind. No user is on any of them.

Note this is the *opposite* of what `UseRollController::destroy()` does today — that soft-deletes the role and hard-deletes the grants. So these were removed by some other path (manual SQL, or an older code version).

**Recommendation: delete the 60 orphaned rows.** They are unreachable — no role, no users — and once `role_permissions.roleId` is constrained with `ON DELETE CASCADE`, this class of garbage cannot recur. Low risk, but it *is* a data deletion, so it needs sign-off.

---

## 3. ON DELETE semantics — reviewed against real workflows

Requirement 5 said to derive these from actual behaviour, not assumptions. **Doing so surfaced a real hazard.**

### The application soft-deletes almost everything

`students`, `users` and `user_roles` are all soft-deleted (`isDelete`), never hard-deleted, with one exception: `ResetFinancialData.php:157` (`DB::table('users')->where('id','!=',1)->delete()`), a developer reset command. So in normal operation **most CASCADE rules will never fire.** That makes restoration low-risk in practice.

### ⚠ But the declared rules are wrong for financial tables

The migrations declare:

| Constraint | Declared rule |
|---|---|
| `invoices.studentId` → `students.id` | **ON DELETE CASCADE** |
| `payments.studentId` → `students.id` | **ON DELETE CASCADE** |
| `programs.studentId` → `students.id` | ON DELETE SET NULL |

So a hard-deleted student would **silently destroy their invoices and payments**, while merely detaching their programs. That is inconsistent, and for financial records it is the wrong behaviour — accounting history should survive the deletion of the entity it refers to.

Today this is inert (students are never hard-deleted). Adding the FK **as declared** would arm it.

**Recommendation:** for the finance batch, override the migration's declared rule and use `ON DELETE RESTRICT` on `invoices.studentId` and `payments.studentId`. A student with financial history should be un-deletable, not silently strip-mined. This is a deliberate divergence from the canonical schema and must be an explicit decision — flag it rather than inherit it.

### Hard-delete sites that CASCADE would newly affect

| Hard-deleted table | Cascades to | Assessment |
|---|---|---|
| `class_schedules` (`ClassSeriesEditService::deleteFutureUnprotected()`) | `attendance_records`, `class_cancellations`, `absence_reports`, `tutoring_planners` | **Behaviour change.** Today these are left as orphans; with FKs they would be cascaded away. `ClassSeriesEditService` already refuses to delete occurrences that *have* attendance or planners (`isProtected`), so in practice the cascade should hit nothing. **Must be tested explicitly** — see §6. All four tables currently hold 0 rows, so there is no back-data to worry about. |
| `role_permissions` (`UseRollController::destroy()`) | — (it is the child) | With `ON DELETE CASCADE` on `roleId`, the manual `DB::table('role_permissions')->where('roleId',$id)->delete()` becomes redundant but harmless. |
| `history_payroll_records` (`moveAllPayrollData()`) | — | No FK proposed on this table. Unaffected. |

`ON UPDATE` is `NO ACTION`/`RESTRICT` throughout, and no workflow updates a primary key. No action needed.

---

## 4. Inventory — 51 missing FKs, grouped into proposed batches

Ordered by dependency (parents before children) and by blast radius (clean/low-risk first).

### FK-Batch 1 — Chat & messaging (8 FKs, 0 orphans)
| Child | Column | → Parent | ON DELETE |
|---|---|---|---|
| `conversations` | *(none missing)* | | |
| `conversation_participants` | `conversation_id` | `conversations.id` | CASCADE |
| `conversation_participants` | `user_id` | `users.id` | CASCADE |
| `chat_messages` | `conversation_id` | `conversations.id` | CASCADE |
| `chat_messages` | `sender_id` | `users.id` | CASCADE |
| `message_reads` | `message_id` | `chat_messages.id` | CASCADE |
| `message_reads` | `user_id` | `users.id` | CASCADE |
| `messages` | `senderId` / `receiverId` | `users.id` | CASCADE |
| `messages` | `parentMessageId` | `messages.id` | SET NULL |

Self-contained, zero orphans, low blast radius. **Best first batch.**

### FK-Batch 2 — Programs / Mastery / Prompt levels (12 FKs, 0 orphans)
`programs` → `program_folders`, `mastery_workflows`, `prompt_level_templates`, `students` (all SET NULL) ·
`program_targets` → `programs` (CASCADE), `mastery_workflows`, `prompt_level_templates` (SET NULL) ·
`prompt_levels` → `prompt_level_templates` (CASCADE) ·
`mastery_workflow_phases` → `mastery_workflows` (CASCADE) ·
`mastery_maintenance_schedules` → `mastery_workflow_phases` (CASCADE) ·
`program_folders` → `program_folders` (self, SET NULL) ·
`session_data_records` → `programs`, `program_targets`, `students`, `teams` (CASCADE)

Mostly SET NULL, zero orphans. Note `program_folders` is still **MyISAM** (T4) — it must be converted first, since InnoDB cannot reference a MyISAM parent.

### FK-Batch 3 — Class operations (13 FKs, 0 orphans)
`class_schedules` → `assign_classes`, `teams`, `students` (CASCADE) ·
`attendance_records` → `class_schedules`, `students` (CASCADE) ·
`class_cancellations` → `class_schedules` (CASCADE) ·
`absence_reports` → `class_schedules`, `students` (CASCADE) ·
`tutoring_planners` → `class_schedules`, `teams`, `students` (CASCADE) ·
`notifications` → `users` (CASCADE) ·
`staff_availability` → `teams` (CASCADE)

**This is the batch that changes runtime behaviour** (the `class_schedules` hard-delete cascade, §3). Several parents are still MyISAM (`absence_reports`, `tutoring_planners`, `staff_availability`, `session_data_records`) and must be converted first.

### FK-Batch 4 — Students & courses (9 FKs, 0 orphans)
`student_notes`, `student_assessments`, `student_challenges`, `student_strengths` → `students` (CASCADE) ·
`assignments` → `courses` (CASCADE) ·
`assignment_submissions` → `assignments`, `students` (CASCADE) ·
`course_materials` → `courses` (CASCADE)

All parents/children still MyISAM — convert first.

### FK-Batch 5 — Finance & RBAC (9 FKs, **62 orphans — BLOCKED**)
| Child | Column | → Parent | Declared | **Recommended** |
|---|---|---|---|---|
| `invoice_items` | `invoiceId` | `invoices.id` | CASCADE | CASCADE (correct — line items belong to the invoice) |
| `transactions` | `account_id` | `accounts.id` | CASCADE | CASCADE |
| `invoices` | `studentId` | `students.id` | CASCADE | **RESTRICT** (see §3) — **1 orphan** |
| `payments` | `studentId` | `students.id` | CASCADE | **RESTRICT** (see §3) — **1 orphan** |
| `role_permissions` | `roleId` | `user_roles.id` | CASCADE | CASCADE — **60 orphans** |
| `role_permissions` | `permissionId` | `permissions.id` | CASCADE | CASCADE |

**Do last.** Requires the orphan decisions in §2 *and* the ON DELETE override decision in §3.

---

## 5. Migration pattern (proposed — not written yet)

Same shape proven across Batches 1–4, extended for FKs:

```php
// PREFLIGHT — refuse rather than corrupt
foreach ($constraints as $c) {
    if (!Schema::hasTable($c->table) || !Schema::hasTable($c->refTable)) continue;   // skip
    if ($this->fkExists($c)) continue;                                                // idempotent

    // 1. both sides must be InnoDB (a MyISAM parent cannot be referenced)
    // 2. child and parent column types must match exactly (incl. signedness)
    // 3. ZERO orphans — throw with the exact offending ids, never silently drop rows
    $orphans = DB::table($c->table)
        ->leftJoin($c->refTable, "{$c->refTable}.{$c->refCol}", '=', "{$c->table}.{$c->col}")
        ->whereNotNull("{$c->table}.{$c->col}")
        ->whereNull("{$c->refTable}.{$c->refCol}")
        ->count();
    if ($orphans > 0) throw new RuntimeException("...{$orphans} orphan(s)...");

    Schema::table($c->table, fn (Blueprint $t) =>
        $t->foreign($c->col)->references($c->refCol)->on($c->refTable)
          ->onDelete($c->onDelete)->onUpdate('restrict'));
}
```

`down()` drops exactly the FKs it added, by name — fully reversible. FKs are **additive**: no data is touched, no rows moved. A rollback restores the previous state exactly.

**One migration per FK-batch.** Never mix an engine conversion and an FK addition in the same migration — a failure would be ambiguous.

---

## 6. Test plan

**With a clean schema** (the rebuilt `stepup_test_db`, which already has all 120 FKs — so these tests are *already* meaningful today):

1. **Deletes** — hard-delete a `class_schedule` with attendance and confirm the cascade; confirm `ClassSeriesEditService` still refuses to delete protected occurrences.
2. **Clone** — `cloneProgram()` under FK enforcement (`promptTemplateId`, `masteryWorkflowId`, `folderId` all constrained).
3. **Payroll** — `moveAllPayrollData()` (no FKs proposed, but confirm no regression).
4. **Attendance / cancellation / release** — the Batch 2 workflows under enforcement.
5. **Chat** — conversation + participants + messages under enforcement (already exercised: `ModuleTransactionTest` had to be given real users precisely *because* the FK fired).
6. **Mastery / prompt levels** — child replacement under CASCADE.
7. **Role deletion** — `UseRollController::destroy()` with `role_permissions.roleId` CASCADE.

**With legacy orphan data** — restore a production snapshot into a scratch DB and run the preflight. It **must fail loudly** on the 62 known orphans rather than silently dropping them. That is the single most important test in this ticket.

---

## 7. Post-deployment verification

```sql
-- 1. Expected FK count (target: 120, matching a fresh install)
SELECT COUNT(*) FROM information_schema.REFERENTIAL_CONSTRAINTS
WHERE CONSTRAINT_SCHEMA = 'stepup_admin_db';

-- 2. Nothing left un-constrained: diff against a fresh install
--    (re-run scripts/fk_inventory.php — expect "MISSING in production: 0")

-- 3. No table left behind
SELECT table_name, engine FROM information_schema.tables
WHERE table_schema = 'stepup_admin_db' AND engine <> 'InnoDB';

-- 4. Re-check orphans (must be 0 for every constrained pair)
```

### Rollback procedure

FKs are additive and carry no data, so rollback is clean:

```bash
php artisan migrate:rollback --path=database/migrations/<fk_batch>.php
```

The `down()` drops each FK it created, by name. No data is restored or lost because none was changed. If a migration fails **mid-batch**, the preflight will have thrown *before* any `ALTER` ran for the offending constraint; constraints added earlier in the same batch remain and are dropped by the rollback.

**Take a backup anyway** — an `ALTER TABLE ... ADD CONSTRAINT` rebuilds the table on MySQL 5.7.

---

## 8. Recommended order

1. **Decide** the three orphan remediations (§2) and the finance `ON DELETE` override (§3). *These are business decisions and block everything downstream.*
2. Convert the remaining MyISAM parents/children needed by FK-Batches 2–4 (T4 + the 19 leftovers) — **an InnoDB child cannot reference a MyISAM parent.**
3. FK-Batch 1 (chat) — cleanest, proves the pattern.
4. FK-Batch 2 (programs/mastery/prompts).
5. FK-Batch 3 (class ops) — the one with a real behaviour change; test hard.
6. FK-Batch 4 (students/courses).
7. FK-Batch 5 (finance/RBAC) — last, after the orphan cleanup.

---

## 9. Open questions for sign-off

1. **`invoices.id=2` / `payments.id=2` → missing student 2.** Placeholder record, NULL, or delete? (Recommend: placeholder — preserve the money trail.)
2. **60 orphaned `role_permissions` rows** for non-existent roles 5/6/7. Delete? (Recommend: yes — unreachable, no users affected.)
3. **Override `ON DELETE CASCADE` → `RESTRICT`** on `invoices.studentId` / `payments.studentId`? (Recommend: yes — financial history must not be silently destroyed by a student delete. This diverges from the migrations, so the migrations should be corrected too, or fresh installs will keep getting CASCADE.)
4. Are the 19 remaining MyISAM tables in scope for conversion as a prerequisite, or should FK-Batches 2–4 be deferred until they are?

---

# ✅ T5 COMPLETE — 51 foreign keys restored

**Production now has exactly 120 foreign keys — identical to a fresh install. Missing: 0.**

| | Before | After |
|---|---|---|
| Foreign keys | 69 | **120** (canonical) |
| Missing FKs | 51 | **0** |
| Orphan rows | 62 | **0** |
| MyISAM tables | 19 | **4** (none required by any FK) |
| `invoices.studentId` ON DELETE | *(no FK)* | **RESTRICT** |
| `payments.studentId` ON DELETE | *(no FK)* | **RESTRICT** |

## ⚠ Correction to this audit's §8

The original plan named **5** prerequisite MyISAM tables. That was wrong: it listed only
prerequisite *parents*. A MyISAM **child** cannot hold a foreign key either, so every MyISAM
child in a missing FK also had to be converted. The true prerequisite set is **15 tables**,
derived from the missing-FK set itself (`scripts/fk_prereq.php`), not from module names.

Only **4** MyISAM tables are genuinely not required by any FK and were left alone:
`activity_logs`, `class_messages`, `class_requests`, `password_otps`. They belong to the
separate remaining-MyISAM ticket.

## Execution record

| Step | Migration | Result |
|---|---|---|
| 1–2. Preflight | `scripts/fk_preflight.php` | **Failed correctly** on all 62 orphans + every MyISAM blocker. **No data modified.** |
| 3. Prerequisites | `2026_07_14_000002_convert_fk_prerequisite_tables_to_innodb` | 15 tables → InnoDB, rows and indexes unchanged |
| 4–5. Orphans | `2026_07_14_000003_remediate_fk_orphan_rows` | Placeholder student **id=69** created; `invoices.id=2` + `payments.id=2` re-pointed; **60** orphaned `role_permissions` deleted |
| 6. Policy | `..._create_invoices_table` / `..._create_payments_table` | CASCADE → **RESTRICT** so fresh installs match |
| 7. FK-Batch 1 | `2026_07_14_000004_add_fks_batch1_chat` | 9 FKs |
| 7. FK-Batch 2 | `2026_07_14_000005_add_fks_batch2_programs` | 15 FKs |
| 7. FK-Batch 3 | `2026_07_14_000006_add_fks_batch3_class_ops` | 13 FKs |
| 7. FK-Batch 4 | `2026_07_14_000007_add_fks_batch4_students_courses` | 8 FKs |
| 7. FK-Batch 5 | `2026_07_14_000008_add_fks_batch5_finance_rbac` | 6 FKs (2 RESTRICT) |
| 8–9. Verify | `scripts/fk_preflight.php` | **PASS — 0 missing** |
| 10. Rollback | `migrate:rollback` | 15 FKs dropped, **all data intact**, re-applied to 120 |

## Orphan remediation — as decided

**Placeholder student id=69** (`reference_id = ARCHIVED-DELETED-STUDENT`):
- `first_name` "Deleted Student", `last_name` "Historical Financial Records", `status` Archived
- `isDelete = 1`, so it is invisible to every query that scopes on `isDelete = false` — pickers,
  lists, reports, exports. **Verified by test.**
- Carries an internal note explaining why it exists and that it must not be deleted.
- `invoices.id=2` and `payments.id=2` re-pointed to it. **Neither was deleted; neither was nulled.**

**60 orphaned `role_permissions`** for non-existent roles 5/6/7: deleted, behind three guards
that re-verify the audit's preconditions against live data and **throw rather than delete**
if production has changed (role resurrected, users attached, or the count is not exactly 60).

The remediation is deliberately **not reversible** — reversing would mean re-orphaning financial
records and re-creating grants for roles that do not exist, i.e. deliberately re-corrupting the
database. The FK migrations that depend on it *are* independently reversible.

## The cardinal rule held

**No FK migration deleted or mutated a single row of production data.** `ForeignKeyMigrator`
throws with the offending ids if it finds orphans; remediation is a separate, signed-off,
guarded migration. That was proven twice: the preflight failed on 62 orphans before
remediation, and the two self-referencing FKs (`messages.parentMessageId`,
`program_folders.parentFolderId`) failed on a join-alias bug **before** any `ALTER` ran —
nothing was left half-applied, and re-running skipped the already-added constraints.

## Tests — `tests/Feature/ForeignKeyIntegrityTest.php` (9 passed, 1 skipped)

| Test | Proves |
|---|---|
| Student with an invoice cannot be hard-deleted | The **database** refuses it; the invoice survives |
| Student with a payment cannot be hard-deleted | Same; the payment survives |
| Soft-deleting a student with financial history still works | The path the app actually uses is unaffected |
| Student **without** financial history can still be hard-deleted | RESTRICT is targeted, not a blanket block |
| Unprotected occurrence can still be deleted | Normal series-edit path still works |
| Occurrence with attendance is protected by the service guard | The guard fires **before** any delete is attempted |
| Deleting an occurrence cascades to dependents | No orphans — the old MyISAM behaviour is gone |
| A blocked delete rolls back completely | Earlier writes in the same transaction unwind; no partial state |
| Every declared foreign key exists | Exactly 120 |
| *(skipped)* Placeholder invisible to normal queries | Only runs where the placeholder exists (production) |

**Full suite: 92 passed, 1 failed, 1 skipped.** The single failure remains the unrelated,
pre-existing `EmailProviderTest` fixture bug (T6).

## `ResetFinancialData` — no change needed

It already wraps everything in `SET FOREIGN_KEY_CHECKS=0` (line 26) and truncates explicitly.
It **never relied on cascading finance deletion**, so the RESTRICT policy does not affect it.
Verified rather than assumed.

## Post-deployment verification

```bash
# Expect: "PREFLIGHT: PASS — all 0 missing FK(s)"
php artisan tinker --execute="require base_path('scripts/fk_preflight.php');"
```
```sql
-- Expect 120
SELECT COUNT(*) FROM information_schema.REFERENTIAL_CONSTRAINTS
WHERE CONSTRAINT_SCHEMA = 'stepup_admin_db';

-- Expect RESTRICT on both
SELECT CONSTRAINT_NAME, DELETE_RULE FROM information_schema.REFERENTIAL_CONSTRAINTS
WHERE CONSTRAINT_SCHEMA = 'stepup_admin_db'
  AND CONSTRAINT_NAME IN ('invoices_studentid_foreign','payments_studentid_foreign');

-- Expect only: activity_logs, class_messages, class_requests, password_otps
SELECT table_name FROM information_schema.tables
WHERE table_schema = 'stepup_admin_db' AND engine <> 'InnoDB';
```

### Rollback

FKs are additive and carry no data. **Tested on the live database:** rolling back FK-Batch 2
dropped its 15 constraints, left `invoices`, `payments` and `role_permissions` row counts
unchanged, and re-applying restored all 120.

```bash
php artisan migrate:rollback --path=database/migrations/2026_07_14_00000X_add_fks_batchN.php
```

Take a backup regardless — `ALTER TABLE ... ADD CONSTRAINT` rebuilds the table on MySQL 5.7.

**Do not roll back `2026_07_14_000003_remediate_fk_orphan_rows`** — it is intentionally a no-op
in reverse (see above).

---

## Post-review update — environment-conditional skip removed

The placeholder test previously **skipped** when the database it ran against had no
placeholder (i.e. on a fresh install), so it only ever asserted on production. That is the
wrong way round for CI — a test that silently skips is not a test.

It now builds its **own** placeholder fixture, mirroring what
`2026_07_14_000003_remediate_fk_orphan_rows` creates, and asserts three things rather than one:

1. The placeholder is a **valid FK target** — an orphaned invoice can be re-pointed at it and
   satisfies `invoices.studentId -> students.id`.
2. It is **invisible** to any query scoping on `isDelete = false`.
3. **RESTRICT still protects the record it carries** — the placeholder itself cannot be
   hard-deleted while it holds an invoice.

`ForeignKeyIntegrityTest`: **10 passed, 0 skipped** (was 9 passed, 1 skipped).

### Final suite status

```
Tests: 93 passed, 1 failed, 0 skipped (464 assertions)
```

**T5 tests pass. The repository suite remains non-green solely because of one unrelated,
pre-existing failure — `EmailProviderTest > duplicate messages are prevented across a re sync`
(tracked as T6 in FOLLOWUP_TICKETS.md).** It is a one-line test-fixture bug in the Email Hub
module: the fixture creates an `EmailThread` without the NOT NULL `subject`. It has no
production impact and is unrelated to the storage-engine, transaction or foreign-key work.