Fell out of the review on #125, which found that a crafted request could save a profile with name = '' and contact_email = '' — and did, against the local stack, to a profile that was already approved and in the directory.
The application half is fixed on that PR: saveProfileAction now takes the draft, runs validateDraft before it writes anything, and maps to the write payload on the server rather than trusting the browser to have done it. This issue is the durable half, which is a migration and did not belong in a PR that carries none.
What is wrong
docs/spec/profile-and-credentials.md says of practitioner_contacts: "contact_email being not null on top of that rules out the address being empty". It does not. not null refuses NULL and accepts '', so the sentence describes a guarantee the schema does not make, and "the enquiry button goes somewhere" — which the same document calls a schema invariant held by the direction of the foreign key — is only as true as whatever last wrote the row.
practitioners.name has the same shape: not null, no check, and it is the one field on a directory row that cannot be absent.
What to do
Two check constraints, in one migration:
practitioner_contacts: check (length(btrim(contact_email)) > 0)practitioners: check (length(btrim(name)) > 0)
btrim rather than a bare comparison, because a single space is an empty answer wearing a value's clothes — the same reasoning blankToNull already applies on the way in.
Correct the spec sentence in the same commit, so the document stops claiming what only this migration makes true.
The trap
Existing rows may already violate both. A local stack that ran the reproduction on #125 has one, and any profile created through the form before that PR could. alter table … add constraint takes an ACCESS EXCLUSIVE lock and validates every row, so it fails outright on a violating row rather than skipping it. Check first, and decide deliberately what happens to a row that fails — an empty name on a published profile is not something to repair silently.
not valid plus a later validate constraint is the usual way to avoid the scan on a large table. That is not the reason to reach for it here — this table is small — but it is the way to land the constraint while a violating row is still being chased.
Fell out of the review on #125, which found that a crafted request could save a profile with
name = ''andcontact_email = ''— and did, against the local stack, to a profile that was already approved and in the directory.The application half is fixed on that PR:
saveProfileActionnow takes the draft, runsvalidateDraftbefore it writes anything, and maps to the write payload on the server rather than trusting the browser to have done it. This issue is the durable half, which is a migration and did not belong in a PR that carries none.What is wrong
docs/spec/profile-and-credentials.mdsays ofpractitioner_contacts: "contact_emailbeingnot nullon top of that rules out the address being empty". It does not.not nullrefusesNULLand accepts'', so the sentence describes a guarantee the schema does not make, and "the enquiry button goes somewhere" — which the same document calls a schema invariant held by the direction of the foreign key — is only as true as whatever last wrote the row.practitioners.namehas the same shape:not null, no check, and it is the one field on a directory row that cannot be absent.What to do
Two check constraints, in one migration:
practitioner_contacts:check (length(btrim(contact_email)) > 0)practitioners:check (length(btrim(name)) > 0)btrimrather than a bare comparison, because a single space is an empty answer wearing a value's clothes — the same reasoningblankToNullalready applies on the way in.Correct the spec sentence in the same commit, so the document stops claiming what only this migration makes true.
The trap
Existing rows may already violate both. A local stack that ran the reproduction on #125 has one, and any profile created through the form before that PR could.
alter table … add constrainttakes anACCESS EXCLUSIVElock and validates every row, so it fails outright on a violating row rather than skipping it. Check first, and decide deliberately what happens to a row that fails — an empty name on a published profile is not something to repair silently.not validplus a latervalidate constraintis the usual way to avoid the scan on a large table. That is not the reason to reach for it here — this table is small — but it is the way to land the constraint while a violating row is still being chased.