fix: altijd de echte serverfout tonen + klasnamen uniek per school (v0.4.1-beta)
All checks were successful
dev - build & deploy naar test / build-and-deploy (push) Successful in 35s
All checks were successful
dev - build & deploy naar test / build-and-deploy (push) Successful in 35s
- de generieke errorhandler gaf alleen detail mee als req.user al gezet was; bleek de fout zelf al vóór/tijdens dat moment te zitten (zoals bij klas verwijderen), dan bleef het toch de kale "serverfout" - nu altijd de echte oorzaak, dit is een intern beheertool zonder publieke bezoekers - klasnamen konden onbeperkt dubbel aangemaakt worden binnen dezelfde school (geen enkele controle) - POST /admin/classes weigert dit nu met 409, en een nieuwe unieke index (school_id, lower(name)) is het echte vangnet - migratie 016 hernoemt eerst bestaande dubbele klasnamen (niets verwijderd) vóórdat de unieke index wordt gezet: anders was de migratie zelf op deze database mislukt en had de app niet meer opgestart
This commit is contained in:
parent
4a31dc9351
commit
6742e07363
5 changed files with 70 additions and 9 deletions
2
VERSION
2
VERSION
|
|
@ -1 +1 @@
|
||||||
0.4.0-beta
|
0.4.1-beta
|
||||||
|
|
|
||||||
28
db/016_class_name_unique.sql
Normal file
28
db/016_class_name_unique.sql
Normal file
|
|
@ -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));
|
||||||
|
|
@ -2,7 +2,7 @@
|
||||||
"use strict";
|
"use strict";
|
||||||
/* version — shown until /api/version resolves (or if the fetch fails, e.g. offline).
|
/* 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. */
|
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(){
|
(function(){
|
||||||
const tag = document.getElementById("verTag");
|
const tag = document.getElementById("verTag");
|
||||||
tag.textContent = "v"+VERSION;
|
tag.textContent = "v"+VERSION;
|
||||||
|
|
|
||||||
20
src/api.js
20
src/api.js
|
|
@ -66,11 +66,12 @@ export default async function api(app) {
|
||||||
app.setErrorHandler((err, req, reply) => {
|
app.setErrorHandler((err, req, reply) => {
|
||||||
if (reply.statusCode >= 400 && reply.statusCode < 500) return reply.send({ error: err.message });
|
if (reply.statusCode >= 400 && reply.statusCode < 500) return reply.send({ error: err.message });
|
||||||
req.log.error({ err }, 'onverwachte serverfout');
|
req.log.error({ err }, 'onverwachte serverfout');
|
||||||
// Voor ingelogde staf (die dit sowieso al kan zien in de detailweergave)
|
// Altijd de echte oorzaak meegeven i.p.v. een kale "serverfout": dit is een
|
||||||
// geven we de echte oorzaak mee i.p.v. een kale "serverfout": anders is
|
// intern beheertool (geen publieke site), en een onherleidbare "serverfout"
|
||||||
// een fout als deze niet te diagnosticeren zonder in de serverlogs te kijken.
|
// was al twee keer een doodlopende weg bij het diagnosticeren van een bug -
|
||||||
const detail = req.user ? `serverfout: ${err.message}` : 'serverfout';
|
// eerder was dit alleen voor ingelogde gebruikers (req.user), maar bleek
|
||||||
reply.code(500).send({ error: detail });
|
// 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 --------------------------------------------------------------
|
// ---- version --------------------------------------------------------------
|
||||||
|
|
@ -370,8 +371,13 @@ export default async function api(app) {
|
||||||
const { name, school } = req.body ?? {};
|
const { name, school } = req.body ?? {};
|
||||||
const schoolId = req.user.allRoles.includes('super') ? school : req.user.school_id;
|
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');
|
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()]);
|
try {
|
||||||
return { class: { id: Number(r.rows[0].id), name: r.rows[0].name } };
|
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) => {
|
app.delete('/admin/classes/:id', async (req, reply) => {
|
||||||
need(req, reply, PERMISSIONS['classes.manage']);
|
need(req, reply, PERMISSIONS['classes.manage']);
|
||||||
|
|
|
||||||
|
|
@ -274,6 +274,33 @@ test('migratie 015 herstelt ON DELETE CASCADE voor user-foreign-keys', async ()
|
||||||
assert.match(sql, /confdeltype NOT IN \('c', 'n'\)/);
|
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 () => {
|
test('hernoemen weigert een lege of te lange naam', async () => {
|
||||||
const { app } = await makeApp(superUser);
|
const { app } = await makeApp(superUser);
|
||||||
const res = await app.inject({ method: 'PATCH', url: '/api/admin/schools/5', cookies,
|
const res = await app.inject({ method: 'PATCH', url: '/api/admin/schools/5', cookies,
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue