Verbeter anatomie en plaatsing van avataraccessoires (v0.4.46-beta) #1

Open
bes-r wants to merge 192 commits from bes-r/avatar-realism into main AGit
7 changed files with 114 additions and 13 deletions
Showing only changes of commit e2ee4820dc - Show all commits

View file

@ -1 +1 @@
0.4.06-beta 0.4.07-beta

View file

@ -0,0 +1,44 @@
-- v0.4.07-beta: schoolbeheerder (en systeemmanager) konden niet aan een klas
-- gekoppeld worden, met een verwarrende "groepsleiding en klas moeten bij
-- dezelfde school horen" - ook al staat dit in de applicatielaag (POST
-- /admin/classes/:id/teachers, src/api.js) al langer voor elke stafrol open.
--
-- Oorzaak: de trigger teach_validate_class_teacher() (uit 006) is destijds
-- geschreven toen alleen groepsleiding aan een klas gekoppeld kon worden, en
-- is nooit meegegroeid toen schoolbeheerder/systeemmanager daar later bij
-- kwamen - de trigger stond dus in de weg van iets wat de app allang toestond.
CREATE OR REPLACE FUNCTION teach_validate_class_teacher()
RETURNS trigger LANGUAGE plpgsql AS $$
BEGIN
IF NOT EXISTS (
SELECT 1
FROM classes c
JOIN users u ON u.id = NEW.user_id
WHERE c.id = NEW.class_id
AND (
-- systeemmanager is school-onafhankelijk, mag overal aan gekoppeld worden
u.role = 'super'
OR (
u.school_id = c.school_id
AND (
u.role IN ('teacher', 'admin')
OR EXISTS (
SELECT 1 FROM user_roles ur
WHERE ur.user_id = u.id AND ur.role IN ('teacher', 'admin')
)
)
)
)
) THEN
RAISE EXCEPTION 'alleen staf uit dezelfde school (of een systeemmanager) kan aan een klas gekoppeld worden';
END IF;
RETURN NEW;
END;
$$;
-- trigger zelf blijft ongewijzigd (verwijst naar de herdefinieerde functie),
-- alleen opnieuw aangemaakt zodat deze migratie op zichzelf leesbaar is
DROP TRIGGER IF EXISTS trg_class_teachers_tenant ON class_teachers;
CREATE TRIGGER trg_class_teachers_tenant
BEFORE INSERT OR UPDATE ON class_teachers
FOR EACH ROW EXECUTE FUNCTION teach_validate_class_teacher();

View file

@ -1276,6 +1276,13 @@
} }
.am-schoolbar-lbl{font-size:16px;} .am-schoolbar-lbl{font-size:16px;}
.am-schoolbar .am-sel{flex:1; min-width:0;} .am-schoolbar .am-sel{flex:1; min-width:0;}
/* de dichtgeklapte " ..."-knop hoort rechts, niet als volle-breedte-rij
links uitgelijnd zoals een gebruikersrij - dat maakt meteen duidelijk dat
het om een aparte actie gaat, niet om nóg een item in de lijst. Eenmaal
uitgeklapt neemt het formulier zelf wél de volle breedte (align-self niet
gezet op .am-add, dus terug naar de stretch van de kolom-flex hierboven). */
.am-add-wrap{display:flex; flex-direction:column;}
.am-add-wrap > .tbtn{align-self:flex-end;}
/* een uitgeklapt toevoegformulier moet nooit op een bestaande-gebruikersrij /* een uitgeklapt toevoegformulier moet nooit op een bestaande-gebruikersrij
lijken: duidelijk eigen kader (accentkleur, stippellijn) i.p.v. de neutrale lijken: duidelijk eigen kader (accentkleur, stippellijn) i.p.v. de neutrale
achtergrond van .am-row/.am-row-tools, plus een titel die herhaalt wát je achtergrond van .am-row/.am-row-tools, plus een titel die herhaalt wát je
@ -1905,7 +1912,10 @@ body.dark .world-fact{background:#1d3b2a;color:#a9e5b7}body.dark .world-fact .wo
#settingsModal .am-list{gap:0; border:1px solid var(--line); border-radius:var(--radius-s); overflow:hidden; background:var(--surface);} #settingsModal .am-list{gap:0; border:1px solid var(--line); border-radius:var(--radius-s); overflow:hidden; background:var(--surface);}
#settingsModal .am-row{ #settingsModal .am-row{
background:transparent; border-radius:0; box-shadow:none; background:transparent; border-radius:0; box-shadow:none;
border-bottom:1px solid var(--line); margin:0; padding:7px 10px; font-size:13.5px; /* meer verticale ruimte dan voorheen (was 7px): rijen zonder gap ertussen
stonden zo dicht op elkaar dat je makkelijk de verkeerde raakte, zeker
op een telefoon */
border-bottom:1px solid var(--line); margin:0; padding:12px 10px; font-size:13.5px;
} }
#settingsModal .am-list > .am-row:last-child, #settingsModal .am-list > .am-row:last-child,
#settingsModal .am-list > .am-add-wrap:last-child{border-bottom:none;} #settingsModal .am-list > .am-add-wrap:last-child{border-bottom:none;}

View file

@ -384,11 +384,15 @@
}); });
tools.appendChild(field("amFieldSchool", ss)); tools.appendChild(field("amFieldSchool", ss));
} }
if(isStaff && can("users.role.change") && u.id!==currentUser.id){ /* rolwijziging: alleen tussen groepsleiding/schoolbeheerder onderling.
/* één functie per gebruiker: alleen de hoofdrol is te wijzigen (super); Systeemmanager is hier bewust nooit een optie - die rol ontstaat alleen
het aparte extra-functies-systeem is uit de UI gehaald */ via de aparte "Nieuwe systeemmanager"-aanmaak (met bevestiging van het
eigen wachtwoord), nooit door een bestaand account te promoveren. Een
bestaande systeemmanager toont dit veld daarom ook niet: er is geen
veilige weg om die rol via dit simpele dropdown-veld te wijzigen. */
if(isStaff && u.role!=="super" && can("users.role.change") && u.id!==currentUser.id){
const rs = h("select","am-sel"); const rs = h("select","am-sel");
["teacher","admin","super"].forEach(r=>rs.appendChild(new Option(T(roleKey(r)), r))); ["teacher","admin"].forEach(r=>rs.appendChild(new Option(T(roleKey(r)), r)));
rs.value = u.role; rs.value = u.role;
rs.addEventListener("change", async ()=>{ rs.addEventListener("change", async ()=>{
try{ await api("/admin/users/"+u.id, {method:"PATCH", body:{role: rs.value}}); reload(); } try{ await api("/admin/users/"+u.id, {method:"PATCH", body:{role: rs.value}}); reload(); }

View file

@ -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.06-beta"; const VERSION = "0.4.07-beta";
(function(){ (function(){
const tag = document.getElementById("verTag"); const tag = document.getElementById("verTag");
tag.textContent = "v"+VERSION; tag.textContent = "v"+VERSION;

View file

@ -661,17 +661,22 @@ export default async function api(app) {
} }
} }
// rolwijziging alleen tussen staf-rollen onderling: leerlingen blijven // rolwijziging alleen tussen staf-rollen onderling: leerlingen blijven
// leerling en ouder-accounts kunnen nooit naar een beheerrol promoveren // leerling en ouder-accounts kunnen nooit naar een beheerrol promoveren.
// 'super' is hier bewust GEEN geldig doel: een systeemmanager aanmaken kan
// alleen via POST /admin/users, met het eigen-wachtwoord-bevestigingsstap -
// niet door een bestaande schoolbeheerder/groepsleiding via deze
// eenvoudige rolwijziging te promoveren. Demoteren van een bestaande
// systeemmanager naar admin/teacher blijft wel mogelijk.
if (b.role && can(req.user.allRoles, 'users.role.change') if (b.role && can(req.user.allRoles, 'users.role.change')
&& ['super', 'admin', 'teacher'].includes(b.role) && ['admin', 'teacher'].includes(b.role)
&& ['super', 'admin', 'teacher'].includes(u.role)) && ['super', 'admin', 'teacher'].includes(u.role))
await pool.query('UPDATE users SET role = $1, school_id = $2 WHERE id = $3', await pool.query('UPDATE users SET role = $1 WHERE id = $2', [b.role, u.id]);
[b.role, b.role === 'super' ? null : u.school_id, u.id]);
// extra functies naast de hoofdrol (bv. een teacher die ook admin-rechten krijgt) - // extra functies naast de hoofdrol (bv. een teacher die ook admin-rechten krijgt) -
// alleen voor staf, zelfde recht als hoofdrol-wijziging; hoofdrol zelf telt niet // alleen voor staf, zelfde recht als hoofdrol-wijziging; hoofdrol zelf telt niet
// ook nog als "extra" // ook nog als "extra". Ook hier is 'super' bewust geen toegestane extra rol,
// om dezelfde reden als hierboven.
if (b.extraRoles !== undefined && can(req.user.allRoles, 'users.role.change') && u.role !== 'pupil') { if (b.extraRoles !== undefined && can(req.user.allRoles, 'users.role.change') && u.role !== 'pupil') {
const extra = [...new Set(b.extraRoles)].filter((r) => ['super', 'admin', 'teacher'].includes(r) && r !== u.role); const extra = [...new Set(b.extraRoles)].filter((r) => ['admin', 'teacher'].includes(r) && r !== u.role);
await pool.query('DELETE FROM user_roles WHERE user_id = $1', [u.id]); await pool.query('DELETE FROM user_roles WHERE user_id = $1', [u.id]);
for (const role of extra) { for (const role of extra) {
await pool.query('INSERT INTO user_roles (user_id, role) VALUES ($1, $2) ON CONFLICT DO NOTHING', [u.id, role]); await pool.query('INSERT INTO user_roles (user_id, role) VALUES ($1, $2) ON CONFLICT DO NOTHING', [u.id, role]);

View file

@ -116,6 +116,33 @@ test('ouder-achtige rol krijgt geen promotie en geen klaskoppeling', async () =>
await app2.close(); await app2.close();
}); });
test('een bestaande groepsleiding of schoolbeheerder kan via rolwijziging nooit systeemmanager worden', async () => {
// zelfs een systeemmanager mag dit niet via de gewone rolwijziging - alleen
// via POST /admin/users (met de eigen-wachtwoord-bevestiging) mag een
// nieuwe systeemmanager ontstaan
const { app, calls } = await makeApp(superUser, { id: 7, role: 'teacher', school_id: 2 });
const res = await app.inject({ method: 'PATCH', url: '/api/admin/users/7', cookies,
payload: { role: 'super' } });
assert.equal(res.statusCode, 200, res.body);
assert.ok(!calls.some((c) => c.sql.startsWith('UPDATE users SET role')),
'promotie naar systeemmanager via rolwijziging moet genegeerd worden');
await app.close();
const { app: app2, calls: calls2 } = await makeApp(superUser, { id: 8, role: 'admin', school_id: 2 });
const res2 = await app2.inject({ method: 'PATCH', url: '/api/admin/users/8', cookies,
payload: { role: 'super' } });
assert.equal(res2.statusCode, 200, res2.body);
assert.ok(!calls2.some((c) => c.sql.startsWith('UPDATE users SET role')));
await app2.close();
// ook niet via het extra-rollen-mechanisme
const { app: app3, calls: calls3 } = await makeApp(superUser, { id: 8, role: 'admin', school_id: 2 });
const res3 = await app3.inject({ method: 'PATCH', url: '/api/admin/users/8', cookies,
payload: { extraRoles: ['super', 'teacher'] } });
assert.equal(res3.statusCode, 200, res3.body);
assert.deepEqual(res3.json().extraRoles, ['teacher']);
assert.ok(!calls3.some((c) => c.sql.startsWith('INSERT INTO user_roles') && c.params.includes('super')));
await app3.close();
});
test('systeemmanager koppelt een gebruiker aan een school en maakt hem weer los', async () => { test('systeemmanager koppelt een gebruiker aan een school en maakt hem weer los', async () => {
// koppelen: school bestaat -> update + klas-/toewijzing-opruiming // koppelen: school bestaat -> update + klas-/toewijzing-opruiming
const { app, calls } = await makeApp(superUser, { id: 7, role: 'teacher', school_id: null, username: 'juf' }); const { app, calls } = await makeApp(superUser, { id: 7, role: 'teacher', school_id: null, username: 'juf' });
@ -296,6 +323,17 @@ test('migratie 016 hernoemt bestaande dubbele klasnamen vóórdat de unieke inde
assert.doesNotMatch(sql, /DELETE FROM classes/); assert.doesNotMatch(sql, /DELETE FROM classes/);
}); });
test('migratie 017 laat schoolbeheerder en systeemmanager ook echt via de database toe aan een klas', async () => {
// regressie: de trigger uit 006 stond alleen 'teacher' toe en blokkeerde
// daarmee (met een verwarrende foutmelding) wat de applicatielaag
// (POST /admin/classes/:id/teachers) allang toestond voor admin/super -
// een mismatch die de gemockte-pool-tests principieel nooit konden vangen.
const sql = await readFile('db/017_class_teacher_roles.sql', 'utf8');
assert.match(sql, /u\.role = 'super'/);
assert.match(sql, /u\.role IN \('teacher', 'admin'\)/);
assert.match(sql, /CREATE OR REPLACE FUNCTION teach_validate_class_teacher/);
});
test('klas aanmaken weigert een dubbele naam binnen dezelfde school (409)', async () => { test('klas aanmaken weigert een dubbele naam binnen dezelfde school (409)', async () => {
const { app } = await makeAppWithSchool(superUser, null); const { app } = await makeAppWithSchool(superUser, null);
const pool = app.pg; const pool = app.pg;