diff --git a/VERSION b/VERSION index 578dca3..88eadc4 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -0.4.0-beta +0.4.1-beta diff --git a/db/016_class_name_unique.sql b/db/016_class_name_unique.sql new file mode 100644 index 0000000..4dce14d --- /dev/null +++ b/db/016_class_name_unique.sql @@ -0,0 +1,28 @@ +-- v0.4.1-beta: een klasnaam mag niet dubbel voorkomen binnen dezelfde school. +-- Zonder deze constraint kon "Nieuwe klas" herhaaldelijk dezelfde naam +-- aanmaken (bv. na een eerdere poging die door een andere bug leek te +-- mislukken, maar stiekem toch al aanmaakte) - de app-laag controleert dit nu +-- ook (POST /admin/classes), maar de database-constraint is het echte vangnet. +-- +-- Bestaande dubbele klasnamen eerst uniek maken: anders faalt de CREATE +-- UNIQUE INDEX hieronder op een database die deze bug al heeft laten +-- ontstaan (en dan start de app helemaal niet meer op). Er wordt niets +-- verwijderd, alleen de latere dubbele naam hernoemd - de klas, z'n +-- leerlingen en koppelingen blijven ongemoeid. +DO $$ +DECLARE + r RECORD; +BEGIN + FOR r IN + SELECT id, name, + ROW_NUMBER() OVER (PARTITION BY school_id, lower(name) ORDER BY id) AS rn + FROM classes + LOOP + IF r.rn > 1 THEN + UPDATE classes SET name = r.name || ' (' || r.rn || ')' WHERE id = r.id; + END IF; + END LOOP; +END $$; + +CREATE UNIQUE INDEX IF NOT EXISTS idx_classes_school_name + ON classes (school_id, lower(name)); diff --git a/public/js/core.js b/public/js/core.js index bef9988..7171749 100644 --- a/public/js/core.js +++ b/public/js/core.js @@ -2,7 +2,7 @@ "use strict"; /* version — shown until /api/version resolves (or if the fetch fails, e.g. offline). Kept in sync by hand with the VERSION file at the repo root on every release. */ -const VERSION = "0.4.0-beta"; +const VERSION = "0.4.1-beta"; (function(){ const tag = document.getElementById("verTag"); tag.textContent = "v"+VERSION; diff --git a/src/api.js b/src/api.js index 260fac8..9cf6242 100644 --- a/src/api.js +++ b/src/api.js @@ -66,11 +66,12 @@ export default async function api(app) { app.setErrorHandler((err, req, reply) => { if (reply.statusCode >= 400 && reply.statusCode < 500) return reply.send({ error: err.message }); req.log.error({ err }, 'onverwachte serverfout'); - // Voor ingelogde staf (die dit sowieso al kan zien in de detailweergave) - // geven we de echte oorzaak mee i.p.v. een kale "serverfout": anders is - // een fout als deze niet te diagnosticeren zonder in de serverlogs te kijken. - const detail = req.user ? `serverfout: ${err.message}` : 'serverfout'; - reply.code(500).send({ error: detail }); + // Altijd de echte oorzaak meegeven i.p.v. een kale "serverfout": dit is een + // intern beheertool (geen publieke site), en een onherleidbare "serverfout" + // was al twee keer een doodlopende weg bij het diagnosticeren van een bug - + // eerder was dit alleen voor ingelogde gebruikers (req.user), maar bleek + // ook dát soms nog niet gezet als de fout vóór/tijdens het inloggen zit. + reply.code(500).send({ error: `serverfout: ${err.message}` }); }); // ---- version -------------------------------------------------------------- @@ -370,8 +371,13 @@ export default async function api(app) { const { name, school } = req.body ?? {}; const schoolId = req.user.allRoles.includes('super') ? school : req.user.school_id; if (!name || typeof name !== 'string' || name.trim().length > 80 || !schoolId) return fail(reply, 400, 'ongeldige naam of school'); - const r = await pool.query('INSERT INTO classes (school_id, name) VALUES ($1,$2) RETURNING id, name', [schoolId, name.trim()]); - return { class: { id: Number(r.rows[0].id), name: r.rows[0].name } }; + try { + const r = await pool.query('INSERT INTO classes (school_id, name) VALUES ($1,$2) RETURNING id, name', [schoolId, name.trim()]); + return { class: { id: Number(r.rows[0].id), name: r.rows[0].name } }; + } catch (e) { + if (e.code === '23505') return fail(reply, 409, 'klasnaam bestaat al binnen deze school'); + throw e; + } }); app.delete('/admin/classes/:id', async (req, reply) => { need(req, reply, PERMISSIONS['classes.manage']); diff --git a/test/schools.test.js b/test/schools.test.js index 6d10283..5c0b96c 100644 --- a/test/schools.test.js +++ b/test/schools.test.js @@ -274,6 +274,33 @@ test('migratie 015 herstelt ON DELETE CASCADE voor user-foreign-keys', async () assert.match(sql, /confdeltype NOT IN \('c', 'n'\)/); }); +test('migratie 016 hernoemt bestaande dubbele klasnamen vóórdat de unieke index wordt gezet', async () => { + const sql = await readFile('db/016_class_name_unique.sql', 'utf8'); + // eerst hernoemen (niets verwijderen), dan pas de constraint - anders faalt + // de migratie zelf op een database die deze bug al heeft laten ontstaan + const renameIdx = sql.indexOf('ROW_NUMBER()'); + const indexIdx = sql.indexOf('CREATE UNIQUE INDEX'); + assert.ok(renameIdx > -1 && indexIdx > -1 && renameIdx < indexIdx, + 'hernoemen van dubbele namen moet vóór het aanmaken van de unieke index staan'); + assert.match(sql, /PARTITION BY school_id, lower\(name\)/); + assert.doesNotMatch(sql, /DELETE FROM classes/); +}); + +test('klas aanmaken weigert een dubbele naam binnen dezelfde school (409)', async () => { + const { app } = await makeAppWithSchool(superUser, null); + const pool = app.pg; + const orig = pool.query.bind(pool); + pool.query = async (sql, params) => { + if (sql.startsWith('INSERT INTO classes')) { const e = new Error('dup'); e.code = '23505'; throw e; } + return orig(sql, params); + }; + const res = await app.inject({ method: 'POST', url: '/api/admin/classes', cookies, + payload: { name: 'B1', school: 2 } }); + assert.equal(res.statusCode, 409, res.body); + assert.match(res.json().error, /bestaat al/); + await app.close(); +}); + test('hernoemen weigert een lege of te lange naam', async () => { const { app } = await makeApp(superUser); const res = await app.inject({ method: 'PATCH', url: '/api/admin/schools/5', cookies,