An approved profile can still carry the review note that rejected it

[davidtaing]

TL;DR

• The problem — only approve_practitioner() deletes a profile's review note, and an admin also holds a direct update (status) grant. Approve that way and the owner reads "your profile is live" and Bluehex's reasons for rejecting it at the same time — review_notes_read_own serves the note to exactly that person.
• The fix — undecided on purpose. The candidates are making reject_practitioner() the only writer of the table, or coupling the note to status; Profile core: contacts, practitioners, review notes, policies, guard and admin RPCs #92 declined to settle it in a trigger, because that would answer the question below by accident.
• To decide — what a review note is: the current reason a profile was refused, or a message to a practitioner. Three cases pull different ways — a profile rejected then approved again, a note written on an already-approved profile, and a withdrawn profile that carries one.

Full context — the reasoning, the constraints and what has already been ruled out. This is the part a coding agent should read.

Fell out of the review on #92. The provenance half landed there; this is the half that is a workflow question rather than a defect.

What happens

approve_practitioner() deletes the profile's row in practitioner_review_notes, on the reasoning that an approved row carries no rejection feedback. Nothing else does. An admin holds update (status) on practitioners — the ownership flow needs that grant and the RPC can never be the only door — so:

PATCH /practitioners?id=eq.… {"status": "approved"}

publishes the profile and leaves the note in place. The owner then reads "your profile is live" and Bluehex's reasons for rejecting it at the same time. review_notes_read_own serves the note to exactly the person it is about, so this is visible to them, not just to an admin.

Why it was not fixed in #92

Coupling the note to status in practitioners_guard means a cross-table write from the trigger whose job is pinning columns on the row in hand, and it fires on every update including the ones that have nothing to do with status.

More to the point, the cases diverge as soon as you look at them, and each wants a different answer:

• a profile approved and later rejected, then approved again
• a note written against a profile that is already approved — feedback that is not a rejection
• a profile withdrawn while it carries a note

That is a question about what a review note is — the current reason a profile was refused, or a message to a practitioner — and the schema currently answers "one current note per profile" without saying which. Deciding it in a trigger would settle it by accident.

Worth deciding together with

• whether reject_practitioner() should be the only writer of the table, which would make the coupling automatic and cost the admin a direct update
• whether a withdrawn profile keeps its note, which is Withdrawal, account deletion, and erasure #52's territory

Where the code is

• supabase/migrations/20260819194255_profile_core.sql — approve_practitioner(), reject_practitioner(), practitioners_guard, review_notes_guard
• tests/db/review-notes.test.ts — the RPC assertions, including deletes the note when an admin approves
• docs/spec/profile-and-credentials.md — the review-note section

Thread: #92 (comment)