Skip to content

feat(registreren): oprichter krijgt school_role='owner' (eigenaar-SSOT) #36

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
onder-ates merged 1 commit into from
Jul 24, 2026
+88 −21
Merged

feat(registreren): oprichter krijgt school_role='owner' (eigenaar-SSOT) #36

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 19 additions & 6 deletions app/api/register-school/route.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -301,23 +301,36 @@ export async function POST(request: NextRequest) {
);
}

// 4. Insert instructor — de registratie-oprichter is de beheerder van de
// nieuwe school en wijkt daarom bewust af van de veilige DB-default
// school_role='employee' (die default beschermt alle overige aanmaakpaden,
// zoals invites).
// 4. Insert instructor — de registratie-oprichter is de eigenaar van de
// nieuwe school (eigenaar-SSOT, ribbaPro-migratie 20260724160000) en wijkt
// daarom bewust af van de veilige DB-default school_role='employee' (die
// default beschermt alle overige aanmaakpaden, zoals invites).
// Bewust géén fallback naar 'admin' als de database 'owner' weigert:
// stil terugvallen zou een school zonder eigenaar aanmaken. Registratie
// faalt dan hard (met volledige cleanup hieronder) — de deploy-volgorde
// eist dat de migratie vóór deze code live is.
const { data: instructor, error: instructorError } = await supabase
.from('instructors')
.insert({
user_id: authUserId,
drivingschool_id: school.id,
status: 'active',
school_role: 'admin',
school_role: 'owner',
})
.select('id')
.single();

if (instructorError || !instructor) {
console.error('Instructor insert error:', instructorError);
if (instructorError?.code === '23514') {
// check_violation — vrijwel zeker instructors_school_role_check die
// 'owner' nog niet kent: ribbaPro-migratie 20260724160000 ontbreekt.
console.error(
'Instructor insert geweigerd door CHECK-constraint. Is ribbaPro-migratie 20260724160000 (school_role owner) toegepast?',
instructorError,
);
} else {
console.error('Instructor insert error:', instructorError);
}
Comment on lines 323 to +333

@coderabbitai coderabbitai Bot Jul 24, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== files =="
git ls-files | rg '(^|/)route\.ts$|migrations|supabase|schema|supabase' | head -200

echo
echo "== target file outline/size =="
wc -l app/api/register-school/route.ts 2>/dev/null || true
ast-grep outline app/api/register-school/route.ts --view expanded 2>/dev/null | sed -n '1,220p' || true

echo
echo "== cleanup/delete related code =="
rg -n "supabase\.|from \('`@supabase`|\.from\(|\.delete\(|\.delete\b|auth|owners?|school|instructor|rollback|cleanup|delete|error\)" app/api/register-school/route.ts | sed -n '1,260p'

echo
echo "== target lines 260-390 =="
cat -n app/api/register-school/route.ts | sed -n '260,390p'

Repository: PolderLabs/ribba-web

Length of output: 10945

🌐 Web query:

Supabase JavaScript Delete returns error object from database operations documentation

💡 Result:

In the Supabase JavaScript client, the delete method follows the standard pattern for database operations: it returns an object containing { data, error, count, status, statusText } [1][2]. Key points regarding the error object and delete operations include: 1. Standard Error Handling: When a database operation fails, the error field will contain a PostgrestError object [3]. This object typically includes a message, code, details, and a hint [3]. It is recommended to log the entire error object rather than just the message, as the hint field often contains the actionable fix provided by Postgres [3]. 2. RLS and Deletion Behavior: A common source of confusion is that a delete operation may appear "successful" (resulting in a null error) even when no rows are deleted [4][2]. This is standard behavior in PostgreSQL: if a Row Level Security (RLS) policy prevents a user from modifying or even "seeing" a row to delete it, the database does not necessarily throw an error [4]. Instead, it may simply return a successful status (e.g., 204 No Content) because the command completed without encountering a technical failure [4][5]. 3. Verifying Deletions: Because delete operations do not return rows by default, they do not provide immediate confirmation that a specific row was removed [1][6]. If you need to confirm that a row was deleted, you can chain the.select method to your delete call [1][6]. If the returned data array is empty, it indicates that no rows were deleted [4]. Additionally, you can check the status and statusText fields returned by the operation to inspect the HTTP response [2]. If you are encountering issues where you suspect a deletion is being blocked by RLS, ensure that your policy allows both the DELETE action and the necessary SELECT permissions (as PostgreSQL often needs to "see" the row to delete it) [4].

Citations:

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== target lines 390-471 =="
cat -n app/api/register-school/route.ts | sed -n '390,471p'

echo
echo "== read-only check: error-destructuring and delete result usage =="
python3 - <<'PY'
from pathlib import Path

p = Path("app/api/register-school/route.ts")
text = p.read_text()

for name, marker in [
    ("school cleanup block", "schoolError || !school"),
    ("instructor cleanup block", "instructorError || !instructor"),
    ("catch cleanup block", "if (authUserId)"),
]:
    idx = text.find(marker)
    start = max(0, text.rfind("\n", 0, idx) - 200)
    end = text.find("\n", idx, idx + 800)
    print(f"[{marker}]")
    for i, line in enumerate(text[start:end].splitlines(), 1):
        if "delete" in line or "error:" in line or "if (" in line:
            print(f"{i:4}: {line}")
    print()

# Extract standalone await delete/deleteUser lines outside matching destructured {error} contexts.
import re
lines = text.splitlines()
for i, line in enumerate(lines, 1):
    stripped = line.strip()
    if not stripped.startswith("await ") or ("delete" not in stripped and ".deleteUser" not in stripped):
        continue
    lookback = text.rfind("\n", 0, text.find(line, text.rfind("\n", 0, i * 80)))
    pre = text[tex t.rfind("\n", 0, lookback) : i * 80] if isinstance(text, str) else ""  # avoid typos?
PY

Repository: PolderLabs/ribba-web

Length of output: 5188

🏁 Script executed:

#!/bin/bash
set -euo pipefail

python3 - <<'PY'
from pathlib import Path

p = Path("app/api/register-school/route.ts")
lines = p.read_text().splitlines()

delete_lines = []
for i, line in enumerate(lines, start=1):
    if "await" in line and ("deleteUser" in line or ".delete(" in line):
        snippet = line.strip()
        before_close = line[:line.find("await")].rstrip()
        after_close = line[line.find("await") + 5:].strip()
        if not before_close and after_close.startswith("const ") and "{ error }" in after_close:
            status = "ERROR_DISCARDED"
        elif not before_close and snip.
PY

Repository: PolderLabs/ribba-web

Length of output: 285

🏁 Script executed:

#!/bin/bash
set -euo pipefail

python3 - <<'PY'
from pathlib import Path

p = Path("app/api/register-school/route.ts")
text = p.read_text()
lines = p.read_text().splitlines()

cleanup_block = text[
    text.find("if (schoolError || !school)") : text.find("if (instructorError || !instructor)")
] + "\n" + text[
    text.find("if (instructorError || !instructor)") : text.find("return NextResponse.json({ success: true })")
]

print("cleanup_blocks_found:")
for query in ["deleteUser", ".delete(", "{ error }", "return NextResponse.json"]:
    print(f" {query}: {cleanup_block.count(query)}")

await_delete_user = [i for i,l in enumerate(lines,1) if "deleteUser" in l and l.strip().startswith("await")]
await_delete_db = [i for i,l in enumerate(lines,1) if ".delete(" in l and l.strip().startswith("await")]
print("await deleteUser line numbers:", await_delete_user)
print("await db delete line numbers:", await_delete_db)

for i in await_delete_user + await_delete_db:
    print(f"{i}: {lines[i-1].strip()}")

# Show context around relevant blocks and any try/catch that wraps them.
targets = []
for start, end in [(294,302),(334,341),(458,464)]:
    targets.append((start, end))
for start, end in targets:
    print(f"\nlines {start}-{end}:")
    for idx in range(start, min(end+1, len(lines)+1)):
        print(f"{idx}: {lines[idx-1]}")
PY

Repository: PolderLabs/ribba-web

Length of output: 1549

Handle cleanup operations that return errors.

Supabase returns deletion failures in { error }, but the cleanup results in the school- and instructor-failure paths are discarded. A failed deleteUser can leave orphaned auth data; a failed school delete at line 335 can leave a school with no corresponding instructor. Check the returned errors durably and reconcile before reporting the rollback as complete.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@app/api/register-school/route.ts` around lines 323 - 333, Handle and persist
the `{ error }` results from cleanup operations in the school- and
instructor-failure paths, including `deleteUser` and the school deletion near
the instructor insert failure. Before reporting rollback as complete, detect
cleanup failures, log or propagate them durably, and reconcile any orphaned auth
user or school record.

// Cleanup
await supabase.from('drivingschools').delete().eq('id', school.id);
await supabase.auth.admin.deleteUser(authUserId);
Expand Down
84 changes: 69 additions & 15 deletions tests/register-school-admin-role.test.mjs → tests/register-school-owner-role.test.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,17 @@
// register-school — pint vast dat de REGISTRATIE-OPRICHTER als school-admin
// wordt aangemaakt (correctie 21 jul 2026). Sinds F3.1A is de DB-default
// voor instructors.school_role de veilige 'employee'; alle rolgevoelige
// domeinen (financiën, Stripe Billing Portal) zijn admin-only. De oprichter
// van een nieuwe school moet die rol dus expliciet meekrijgen — anders is
// een via de web geregistreerde school direct beheerder-loos. Invite-paden
// register-school — pint vast dat de REGISTRATIE-OPRICHTER als eigenaar
// (school_role='owner', eigenaar-SSOT sinds ribbaPro-migratie 20260724160000)
// wordt aangemaakt. Sinds F3.1A is de DB-default voor instructors.school_role
// de veilige 'employee'; alle rolgevoelige domeinen (financiën, Stripe
// Billing Portal) zijn admin-niveau ('owner'/'admin'). De oprichter van een
// nieuwe school moet de eigenaarsrol dus expliciet meekrijgen — anders is
// een via de web geregistreerde school direct eigenaar-loos. Invite-paden
// blijven buiten dit bestand: die maken hun instructeursrijen in ribbaPro
// aan en behouden daar hun bestaande rolgedrag (default employee).
// aan en behouden daar hun bestaande rolgedrag (default employee; invites
// kunnen nooit een owner opleveren).
//
// Pint daarnaast het fail-closed-gedrag vast: weigert de database 'owner'
// (CHECK-constraint, migratie niet toegepast), dan faalt registratie hard
// mét volledige cleanup — géén stille fallback naar 'admin'.

import { test, mock } from 'node:test';
import assert from 'node:assert/strict';
Expand All @@ -16,6 +22,9 @@ process.env.NEXT_PUBLIC_BASE_URL = 'https://preview.test';
delete process.env.RESEND_API_KEY; // e-mailpad slaat dan netjes over

const inserts = []; // { table, payload }
const deletes = []; // { table, column, value }
const deletedAuthUsers = []; // user ids
let failInstructorInsertWith = null; // Postgres-error om instructor-insert mee te laten falen

function makeFakeSupabase() {
return {
Expand All @@ -28,7 +37,10 @@ function makeFakeSupabase() {
},
error: null,
}),
deleteUser: async () => ({ error: null }),
deleteUser: async (id) => {
deletedAuthUsers.push(id);
return { error: null };
},
},
},
from(table) {
Expand All @@ -39,18 +51,26 @@ function makeFakeSupabase() {
}),
insert(payload) {
inserts.push({ table, payload });
const failThis = table === 'instructors' && failInstructorInsertWith;
return {
// pad mét .select('id').single() (drivingschools/instructors)
select: () => ({
single: async () => ({
data: { id: `${table}-1` },
error: null,
}),
single: async () =>
failThis
? { data: null, error: failInstructorInsertWith }
: { data: { id: `${table}-1` }, error: null },
}),
// pad dat direct ge-await wordt (licenses/invitation_links)
then: (resolve) => resolve({ data: null, error: null }),
};
},
// cleanup-pad: .delete().eq('id', ...) wordt direct ge-await
delete: () => ({
eq: (column, value) => {
deletes.push({ table, column, value });
return Promise.resolve({ error: null });
},
}),
};
},
};
Expand Down Expand Up @@ -83,6 +103,13 @@ mock.module('@/lib/legal-acceptances', {

const { POST } = await import('../app/api/register-school/route.ts');

function resetState() {
inserts.length = 0;
deletes.length = 0;
deletedAuthUsers.length = 0;
failInstructorInsertWith = null;
}

function makeRequest() {
return {
headers: { get: () => null },
Expand All @@ -104,14 +131,14 @@ function makeRequest() {
};
}

test('registratie-oprichter wordt aangemaakt met school_role=admin', async () => {
inserts.length = 0;
test('registratie-oprichter wordt aangemaakt met school_role=owner', async () => {
resetState();
const res = await POST(makeRequest());
assert.equal(res.status, 200);

const instructorInserts = inserts.filter((i) => i.table === 'instructors');
assert.equal(instructorInserts.length, 1); // precies één instructeursrij
assert.equal(instructorInserts[0].payload.school_role, 'admin');
assert.equal(instructorInserts[0].payload.school_role, 'owner');
assert.equal(instructorInserts[0].payload.user_id, 'auth-user-1');
assert.equal(instructorInserts[0].payload.status, 'active');

Expand All @@ -123,3 +150,30 @@ test('registratie-oprichter wordt aangemaakt met school_role=admin', async () =>
assert.equal(inviteInserts[0].payload.invite_type, 'student');
assert.equal('school_role' in inviteInserts[0].payload, false);
});

test('CHECK-weigering van owner → harde 500 mét cleanup, geen fallback naar admin', async () => {
resetState();
failInstructorInsertWith = {
code: '23514',
message:
'new row for relation "instructors" violates check constraint "instructors_school_role_check"',
};

const res = await POST(makeRequest());
assert.equal(res.status, 500);

// Eén poging met 'owner', géén tweede insert met 'admin' als fallback
const instructorInserts = inserts.filter((i) => i.table === 'instructors');
assert.equal(instructorInserts.length, 1);
assert.equal(instructorInserts[0].payload.school_role, 'owner');

// Volledige cleanup: school verwijderd én auth-user verwijderd
const schoolDeletes = deletes.filter((d) => d.table === 'drivingschools');
assert.equal(schoolDeletes.length, 1);
assert.equal(schoolDeletes[0].value, 'drivingschools-1');
assert.deepEqual(deletedAuthUsers, ['auth-user-1']);

// Geen vervolg-writes na de mislukte instructeur (license/invite-link)
assert.equal(inserts.filter((i) => i.table === 'instructor_licenses').length, 0);
assert.equal(inserts.filter((i) => i.table === 'invitation_links').length, 0);
});