Skip to content

Migrations vs Actions for Onboarding Defaults ​

This document preserves the order of reasoning, not just the conclusion. Each section is a question that seemed promising, followed by what checking it against the actual codebase revealed. Several early conclusions get overturned later — that's kept visible rather than cleaned up, because the sequence of "seems safe → actually isn't → here's why" is the reusable part for the next person who has the same idea.

Bottom line, if you only read one section: Recommendation at the end. Short version: not worth it — skip to there before reading the rest if you just want the answer.

1. The original idea: replace auto-seed Actions with migrations ​

Trigger: SeedDefaultCareTypesAction doesn't insert rows — care_types already exist from a migration (2024_02_12_110913_add_care_types_table.php), the Action only flips active to true. So why not just seed everything via migrations and skip the Action layer entirely?

First-pass objection: tenant migrations aren't scoped to "only new tenants." customers:migrate defaults to running against every tenant in the central customers table (Tenancy::runForMultiple(), vendor/stancl/tenancy/src/Tenancy.php:138-163), so a migration re-run against an existing tenant could reinsert/reactivate data a tenant deliberately changed.

2. How does customers:migrate actually work? ​

Verified before going further, rather than assumed:

  • Each tenant database gets its own standard Laravel migrations table — the same mechanism the central DB uses, just scoped per-tenant connection. This is framework behavior (Migrator::createRepository()), not a stancl/tenancy concept.
  • New tenant provisioning: ProvisionCustomerAction → stancl's MigrateDatabase job calls Artisan::call('tenants:migrate', ['--tenants' => [$tenant->getTenantKey()]]) (vendor/stancl/tenancy/src/Jobs/MigrateDatabase.php:32-37). Since the new tenant's DB has no migrations table yet, every migration in database/migrations/customer/ runs from scratch.
  • Existing tenants: customers:migrate (backend/app/Console/Commands/CustomerMigrate.php) defaults to ALL tenants if no --customers filter is passed. Each tenant's own migrations table means only pending (not-yet-recorded) migrations run there — already-applied ones are skipped, per tenant, automatically. This is NOT automatic on deploy — nothing in config/tenancy.php or the app schedules it; a human/CI step must run it explicitly.

Correction to the first-pass objection: the "runs on every deploy" fear was overstated — it only fires when someone deliberately runs the fleet-wide command, and even then, once per tenant, ever (tracked by that tenant's own ledger). This weakens the scheduling objection but does not resolve the real one below.

3. The real blocker: mutation after the fact ​

A per-tenant migration ledger answers "has migration X run against tenant Y" — it has no way to know whether the data that migration inserted still looks the way it did on insert. For a brand new tenant this is moot. For an existing tenant, if someone runs customers:migrate fleet-wide after a tenant has deactivated a care type or edited a mentor type, the migration still runs (never run there before) and blindly re-inserts/re-activates rows — clobbering the customization.

This reframes the test for "is X a safe migration candidate" as two conjunctive conditions:

  1. Unconditional — no per-customer variation, no ordering dependency on other pipeline steps.
  2. Never mutated by the tenant afterward — nothing downstream can legitimately change it.

Mentor types and care types were assumed to satisfy both. Neither does (see §7).

4. Detour: what does "run for every tenant" actually cost, mechanically? ​

Walked through what a hypothetical SeedDefaultMentorTypesAction-as-migration would do, using the real Action as the anchor (backend/app/Actions/Model/CustomerOnboarding/SeedDefaultMentorTypesAction.php):

  • For a new tenant: functionally identical outcome to the Action — same 4 rows, same insert, same timing relative to CreateAdminUserAction (which requires ≥1 mentor type to exist).
  • What's lost: the paranoid re-verification (count() !== count($cases) check — guards against save() silently returning false), transaction membership in SetupCustomerAction's single transaction (EMMIE-0470), and Mockery-based unit testability.
  • For an existing tenant retroactively migrated: no way to distinguish "never had this data" from "tenant customized it since" — the disqualifying property from §3, restated concretely.

Conclusion: for new tenants, a migration and an Action produce the same end state with weaker self-verification. The reason mentor types specifically stay disqualified isn't "the migration wouldn't work for new tenants" — it's that the same file, if later run against existing tenants (which this codebase's tooling allows with zero guardrail), can't tell the two histories apart.

5. Can migrations contain if statements? (conditional/scoped migrations) ​

Yes — up()/down() are plain PHP methods. Two different uses of this were considered:

Schema-safety idempotency guards (if (!Schema::hasColumn(...))) — fine, normal, not what's being asked here.

Tenant/data-shape conditionals (branching on which customer this is or what the onboarding payload contained) — this is pipeline logic wearing a migration costume. A migration has no concept of "this specific tenant's request data" the way SetupCustomerAction does, and a migration failing mid-if doesn't get graceful "log a warning, continue" treatment — it throws and aborts the whole MigrateDatabase job, with VerifyDatabaseMigrationAction's table-count check unable to notice a bad data write (it only counts tables).

6. "Repair" migrations — a real precedent that looked relevant but wasn't ​

Found a family of existing migrations doing exactly this: 2026_05_13_000000_repair_schedule_monday_sunday_alignment.php and siblings (ADR-0023's "forward-only chokepoint" amendment, backend/CLAUDE.md). These unconditionally run for every tenant (including brand-new ones with zero rows) but are safe because their candidate-selection query is scoped to an objectively wrong state (WEEKDAY(start_date) <> 0 — either true or false, no judgment call, nothing legitimate ever produces a matching row).

Why this doesn't transfer to onboarding defaults: "zero rows in care_legislations" is not objectively wrong the way a misaligned date is — it's satisfiable both by "never seeded" (the bug) and "tenant deliberately emptied it" (a legitimate choice). A repair migration's safety comes from the predicate being provably corruption, not from "runs unconditionally for everyone." This pattern is the right tool for cleaning up a specific shipped bug's blast radius after the fact (e.g. "any tenant created in the 3-week window before RelationshipTypes was fixed, with zero relationship types, gets the 5 defaults") — not a general "seed defaults" mechanism.

7. Reality check: is anything on the epic list actually immutable? ​

Went through the epic's list item by item, initially assuming several were "fixed forever":

  • Care legislations — assumed fixed-enum-only. Wrong — CareLegislationController exists; fully user-editable (confirmed by the user, then verified: a live controller). Disqualified.
  • Mentor types / care types — already known disqualified (tenant-editable surfaces exist, removed from the wizard but not from day-to-day tenant management).
  • Relationship types (not yet built) — "can be added later" per the user; exists specifically because CreateIntakeContactAction hard-crashes without at least one persoonlijk begeleider row (BreakingOnboarding.md Round 2). Same shape as mentor types: guarantee-at-least-N-exist, tenant may add more, disqualified from being a plain migration for the same reason.
  • Document categories — already seeded via an existing migration (2024_12_30_122924_add_document_category_entries.php), initially cited as validating precedent. Also wrong — the user confirmed these can be added to later too, meaning the existing precedent sits on the same disqualifying property; it just hasn't caused a visible problem yet (BreakingOnboarding.md's "document categories are legislation-agnostic" finding is exactly this rough edge surfacing).
  • Terminology — one row, but editable immediately (that's the entire feature). Disqualified.
  • Paid modules — not reference-data seeding at all; a central-DB billing/subscription flag (messages_enabled). Not a migration-vs-Action question.

Revised conclusion: there is no genuinely immutable seed candidate anywhere in this epic's current or planned list. Every reference-data table a tenant can plausibly touch later — which turned out to be all of them — fails the mutation test. This is stronger and more useful than the original per-item guesswork, precisely because the guesses were wrong until checked.

8. Attempts to route around the mutation problem ​

Several variations were tried, each closing part of the gap but reintroducing the same cost:

"Migration only for tenants created after a specific date" — mechanically works (Customer::find(tenant('id'))->created_at->lt(...)) and does correctly separate pre-cutover tenants (who never had the data, nothing to have customized) from post-cutover ones. But the moment the migration contains this if, it has become an Action with worse tooling: no paranoid re-verification, no transaction membership in SetupCustomerAction, no DI, no normal unit test, plus a permanent no-op date-check every future tenant will silently re-evaluate forever. Net: closes the mutation-safety gap, doesn't deliver the original motivation (reducing risk to customer creation) better than the Action already does.

"Edit the already-run migration file directly, since it won't re-run for existing tenants anyway" (e.g. flip care_types' seeded active default to true, or do the same for paid modules). Mechanically true — a tenant's migration ledger means editing an already-applied file only affects future tenants. But this breaks a rule for a real, independent reason beyond "the rule says not to": migration files are supposed to be an immutable historical record. Editing one after real tenants have run it creates a silent, permanent divergence between what the file says and what actually happened for every tenant that predates the edit — migrate:fresh (dev, CI, test RefreshDatabase) now produces a different state than production tenants of the same "vintage," and down()/rollback semantics diverge from what actually shipped. Checked without the project's own "don't touch existing migrations" rule in place — the conclusion holds independent of that rule; the rule just makes the answer faster to reach.

Already-solved side-note: SeedDefaultCareTypesAction already achieves "new tenants get active care types" today, without editing the migration — there was no missing capability motivating this attempt, only a wish to delete the Action.

"One single DumpOnboardingCriticalDataAction instead of many leaf Actions" — doesn't change the operation count (same inserts, same transaction, same ability to fail), only removes function boundaries between them. Costs: loses per-concern test isolation (directly contradicts the epic's own stated "Bundling of integration tests" principle — SetupCustomerAction vs SeedOnboardingDefaultsAction are deliberately split because "did we sequence/rollback correctly" and "did we seed the right data" are different concerns); loses scoped exception messages (today, "Mentor types were not persisted..." vs a generic monolith failure); loses independent addressability for future one-leaf-at-a-time gutting tickets; fights the 5-constructor-dependency cap that exists specifically so each dependency can be independently mocked. Concluded: not safer, same risk, worse diagnosability.

9. What if we check empty-table, then seed? ​

The one idea that survived. Framed precisely: is "table has zero rows" a safe signal that this tenant was never seeded (as opposed to "tenant deliberately emptied it")? This only works if no legitimate path to empty exists — otherwise the predicate silently resurrects deliberately-deleted data with no error and no log an admin would ever see.

Checked per table, with file:line citations, rather than assumed:

TableCan reach empty via legitimate use?Verdict
terminologyNo delete route exists at all — TerminologyController::update() only does WHERE phrase = ... firstOrFail()->update(...), pure UPDATE, never INSERT/DELETE.Safe. Empty can only mean never-seeded.
settingsNo delete route/action exists anywhere. SaveSettingsAction inserts the one row exactly once (newInstance()->save()); UpdateGeneralSettingsAction only ever mutates it.Safe. Also the highest-value case — BreakingOnboarding.md documents a missing settings row as a hard login crash (No query results for model [Settings]).
document_categories (the 10 seeded rows)DeleteDocumentCategoryAction throws UneditableException for any ID matching DocumentCategoryEnum — exactly IDs 1–10, the seeded set. Permanently undeletable regardless of document references.Safe, scoped to the 10 defaults specifically (a tenant can still have zero custom, id>10, categories — irrelevant to this predicate).
mentor_typesDeleteMentorTypeAction guards on "no users currently assigned to this type," not on "at least one type must exist." A tenant with zero users assigned to any mentor type can legitimately delete all of them.Unsafe. Guard doesn't prevent the state this predicate assumes can't happen.
care_legislationsSoft-deletes (SoftDeletes trait), no floor guard equivalent to LastActiveCareTypeException. A naive COUNT(*) never reaches 0 (trashed rows still count) — predicate silently never fires. Scoped to whereNull('deleted_at'), it fires the moment a tenant deactivates their only legislation — with zero guard against that being deliberate.Unsafe either way: wrong query never fires, right query fires on legitimate tenant action.
provided_care_types (care types)No delete route exists; active can never go below 2 (LastActiveCareTypeException, confirmed in UpdateProvidedCareTypeAction). Row count can never reach 0 in the first place.Moot — the predicate is never needed because empty is structurally unreachable.

Net result: Terminology, Settings, and the 10 default Document Categories all pass. This is a materially better outcome than the original "just terminology" first guess — it reaches roughly half the epic's critical/non-critical seed surface, and specifically covers settings, which has the worst documented failure mode of anything in this epic (total login lockout, not just a cosmetic English-fallback).

A caveat surfaced during this check, worth keeping: a guard can be added later by an unrelated change (someone adds a "must keep at least one X" constraint next year) without the person adding it knowing an empty-table repair predicate depends on that table's current shape. Any implementation of this pattern should document, next to the guard-dependent code, that removing/weakening the guard also invalidates the empty-check predicate.

What actually reduces risk to customer creation (the real thread underneath all of this) ​

The original stated goal (not "fewer files," but "defaults shouldn't put creation of a new customer at risk") turned out to have a sharper lever than migrations at all: criticality misclassification, evidenced directly by docs/onboarding/BreakingOnboarding.md:

  • settings is currently non-critical (log-and-continue) but empirically causes total login lockout if skipped — a mismatch between classification and actual blast radius.
  • terminology is currently non-critical but produces a silent, unrecoverable-without-DB-access broken state (English fallback + an edit modal with no way to add rows).

Confirmed against live source: settings is a landmine, not (yet) a live bug ​

Checked SetupCustomerAction.php:81-90 directly rather than trust the doc summary:

php
if ($payload->settings instanceof SaveSettingsDto) {
    $this->saveSettings->execute($payload->settings);
    // Non-critical silent failure. Default values exist and seem sensible. Doesn't break functionality.
    if (!$this->settings->newQuery()->exists()) {
        $this->logger->warning('Settings were not persisted to database during onboarding.', [...]);
        $hadWarnings = true;
    }
}

Two things are true at once here, and it's important not to conflate them:

Today, this is probably correctly classified. The "non-critical" comment almost certainly dates from when the settings wizard step was still live and always supplied a SaveSettingsDto — so the $payload->settings instanceof SaveSettingsDto gate was, in practice, always true, and "non-critical" meant "if the row-write itself fails, don't abort provisioning over a user's toggle choices." That's a defensible call: losing specific settings values doesn't break functionality, the row still gets created with sensible defaults on retry/next attempt.

The landmine: this is exactly the pre-gutting shape that mentor types, care types, and terminology were all in before their own tickets landed. Each of those was "non-critical, gated on a wizard payload that's assumed to always exist" — right up until the gutting ticket removed the wizard step and the assumption silently stopped holding. Settings has not been gutted yet (the epic's ticket list has no generic "Settings" entry — only send_birthday_emails_settings, called out as needing to move off SaveSettingsDto specifically), so $payload->settings is presumably still always populated today and this code path is not currently reachable in its dangerous form.

Why it's still worth flagging now, before it's a live bug: if/when a future ticket guts the settings wizard step the way the other three were gutted, $payload->settings becomes nullable in practice, and at that point:

  1. The if block — including the warning log — stops executing entirely. Not "logs a warning and continues," but "does nothing, and nothing anywhere records that it did nothing."
  2. Per BreakingOnboarding.md, a genuinely missing settings row causes No query results for model [App\Models\Customer\Settings] — a hard crash on login, for every user. That's a categorically worse failure mode than terminology's degrade-to-English, and directly contradicts the comment's own "Doesn't break functionality" — a claim that was true under the old wizard-always-populates assumption and would stop being true the moment that assumption breaks.

The actionable takeaway: whenever a Settings-gutting ticket is proposed, it should not just follow the mentor-types/care-types/terminology playbook (move to unconditional auto-seed) by rote — it should specifically verify the missing-row failure mode is closed, either by promoting seeding to critical/unconditional (like seedOnboardingDefaults) or by adding a defensive guard wherever Settings is read at runtime (e.g. login) so a missing row degrades gracefully instead of crashing. This is independent of, and stacks with, §9's finding that settings passes the empty-table safety check (no delete route exists anywhere, single row inserted once) — meaning an empty-check-then-seed guard is a safe, available tool for whichever ticket eventually does this gutting.

Checked: does any migration provide a safety net here? No — confirmed gap, not a theoretical one ​

Prompted by "which migrations touch settings, is this data ever written by anything other than SaveSettingsAction" — worth checking directly rather than assuming a migration might quietly backstop this. It doesn't.

Three migrations touch the settings table:

  • 2021_11_02_132900_settings_table.php — creates the original key-value shape. No data insert.
  • 2025_03_10_122926_change_settings_table.php — restructures to the current named-columns shape. It does contain an insert (DB::table('settings')->insert([...])), but gated on count($settings) > 1 — a carry-forward for existing tenants that already had ≥13 rows in the old key-value shape. For a brand-new tenant, the old-shape table is empty, the count check is false, and no row is inserted — this migration is schema-only for new tenants.
  • 2026_04_20_000000_add_default_intake_contact_relation_type_to_settings.php — adds one nullable column and runs an unconditional DB::table('settings')->update([...]) with no WHERE clause. For a new tenant with zero existing rows at this point in the pipeline, update() against zero rows is a no-op.

Conclusion: for a brand-new tenant, no automatic mechanism ever inserts a settings row.SaveSettingsAction is the sole code path that creates it during real provisioning. This is a stronger and more concrete version of the landmine finding above — it's not "there's a risk if X happens," it's "if SaveSettingsAction doesn't run or its save() silently fails, no other mechanism in the provisioning pipeline, now or on any future customers:migrate run, will ever create this row." The table stays permanently empty until someone notices via a login crash.

Correction (crit review, 1c5bd0d8ae0a): a manual recovery mechanism does exist and it does work against production. Database\Seeders\Customer\SettingSeeder::run() inserts a row when Settings::count() === 0, and it carries no environment check of its own. Run directly as php artisan customers:seed --customers=<id> --class=SettingSeeder, it resolves and executes via CustomerSeed::getSeeder() (backend/app/Console/Commands/CustomerSeed.php:104-133) — a path that never touches TenancySeeder or DatabaseSeeder, the two wrapper seeders that gate themselves off production (TenancySeeder::run()'s isProduction() refusal; DatabaseSeeder::run()'s local/testing/staging scope). Those wrapper guards protect their own call sites (spin-up-a- throwaway-dev-tenant, demo-data-fill) — they say nothing about invoking SettingSeeder by name, which is exactly the break-glass path an operator would use.

So the claim above is narrower than first stated: no automatic mechanism in the provisioning pipeline creates this row, but a one-line manual command already exists and already works in production if someone reaches for it by name. §9's empty-check-then-seed guard is still the only automatic safety net for this case — worth building proactively so a locked-out admin isn't gated on someone remembering this command exists, but it would not be closing a gap where zero recovery tooling exists today.

This was flagged directly by the user mid-investigation as "a real mistake" in how settings was originally scoped/classified — worth keeping attached to this finding as the reason it's here at all.

Both are also now candidates for the empty-check-then-seed defensive pattern from §9, which is a different, complementary fix from reclassifying criticality inside SetupCustomerAction — the empty-check could live wherever these values are read, not just at provisioning time, closing the gap even for tenants that already exist in this broken state today.

See docs/onboarding/BreakingOnboarding.md for the full empirical catalogue this was checked against, and docs/onboarding/OnboardingRevamp.md for the epic's ticket list and standing conventions (including "don't touch existing migrations," which §8 independently re-derives from first principles).

Recommendation ​

Not worth pursuing. Of every approach tried (§1–§8), exactly one — §9's empty-table guard — survives, and it only survives for three tables (terminology, settings, the 10 default document_categories) — and even among those three, only one is actually protected by design. That's not "migrations are a viable alternative to Actions here" — it's "one narrow, incidental loophole exists, discovered by auditing each table's delete/deactivate path one at a time."

Worth being precise about why each of the three passes, because it's not the same reason:

TableWhy it's safeIs this a designed guard?
document_categoriesDeleteDocumentCategoryAction explicitly checks the row's ID against DocumentCategoryEnum and throws UneditableException for the 10 seeded IDs. A delete route exists — the Action underneath deliberately refuses.Yes. Someone decided these specific rows must never be deletable.
terminologyNo delete route or Action exists anywhere in the codebase.No. Nobody decided this must stay non-empty — nobody has built a delete feature for it, full stop.
settingsNo delete route or Action exists anywhere in the codebase.No. Same as terminology — absence of a feature, not a rule.

So the real count is 1 out of 3 with an actual invariant behind it. The other two are safe only because a delete button was never built — which is exactly the kind of thing that changes without anyone realizing it's load-bearing. terminology in particular is under active rework in this same epic; a future ticket adding delete-a-term (plausible — the terminology UI is already being touched) would silently invalidate the empty-check predicate for that table, with no test anywhere that would catch it, because nothing today documents that the predicate depends on that absence.

Three reasons to stop here rather than build on it:

  1. A migration is a worse Action, not a better one. Even in the one case where the empty-check pattern works, it gives up paranoid re-verification, transaction membership in SetupCustomerAction's single transaction (EMMIE-0470), constructor-injected dependencies, and normal Mockery-based unit testing (§4) — for no gain over the Action that already does the job.
  2. The opportunity is tiny, and getting tinier. Three tables, out of the whole epic's seed surface, qualify — and all three are already handled by existing Actions today with no known bug. The only thing this would change is where the insert happens, not whether it happens or how safely.
  3. Two of the three qualify by accident, not by design. Per the table above, only document_categories has an actual invariant behind it. terminology and settings pass only because nobody has built a delete feature for them yet — an absence, not a rule. Building a migration strategy on that means the codebase ends up with two different mechanisms doing the same conceptual job (Actions seed most tables, migrations seed a handful chosen because a delete button happens not to exist yet), a distinction future readers have no way to infer from the code itself, and — for 2 of the 3 tables — one unremarkable future PR away from silently resurrecting deleted tenant data with no test anywhere positioned to catch it.

What to do instead: keep using Actions for seeding, as the epic already does. If the actual goal is "reduce risk to customer creation" (the goal that started this detour), the higher-leverage lever is the criticality-misclassification thread this doc surfaced along the way — see "What actually reduces risk to customer creation" above. settings in particular is worth a defensive read-time guard regardless of what happens with this migrations question.