fix: address PR #2 review findings across backend and frontend

- Makefile: add SHELL := /bin/bash so test-env.sh pipefail works in CI
- RoomCanvas: fix onElementClick firing on drag start (now fires on mouseup
  for click-in-place only); fix Props type to accept null; guard grid pattern
  against snapStep=0 (invalid SVG); remove unsafe null cast
- live/[slotId]: fix studentNamesBySeat $derived wrapping a function instead
  of a value — reactivity was broken, map never updated
- s/[code]: block clicks on occupied seats before hitting the backend;
  pass occupiedSeatIds to confirmed-view RoomCanvas; clear errorMsg on retry
- rooms/+page: replace alert() in deleteRoom with inline errorMsg state
- rooms/[roomId]: replace deprecated .substr with .slice
- courses.rs: assign_tutor uses fetch_optional → 404 on unknown tutor_id
  instead of propagating RowNotFound as 500
- rooms.rs: delete_room returns 404 when room does not exist; replace
  fract() != 0.0 float check with epsilon-based validation
- auth_routes.rs: refresh endpoint re-checks is_active so deactivated tutors
  cannot obtain new access tokens; fix test INSERT to include is_active
- tutors.rs: wrap delete_tutor reference checks and DELETE in a transaction
- attendance.rs: replace #[allow(clippy::type_complexity)] with type alias
- migrations/003: document > 50 heuristic precondition

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Makefile

@@ -1,3 +1,5 @@
+SHELL := /bin/bash
+
 .PHONY: dev dev-backend dev-frontend build test compose-up seed-demo \
         test-up test-down test-reset test-rebuild test-e2e
 

backend/migrations/003_normalize_room_layout_units.sql

@@ -1,6 +1,9 @@
 -- Normalize room layout units: divide pixel-scale coordinates by 40.
 -- Applied per element with CASE WHEN so the transform is idempotent:
 -- coordinates already in grid units (≤50) are left untouched.
+-- PRECONDITION: assumes no pre-existing room has pixel-scale coords ≤50.
+-- Any element with pixel x/y/width/height ≤50 will be skipped (treated as
+-- already converted). Verify this holds before running on a production DB.
 UPDATE rooms
 SET layout_json = (
     SELECT json_group_array(

backend/src/routes/attendance.rs

@@ -79,9 +79,18 @@ async fn get_session_attendance(
     .fetch_all(&pool)
     .await?;
 
-    // Get all slots for the session with their layouts
-    #[allow(clippy::type_complexity)]
-    let slot_rows: Vec<(i64, i64, Option<i64>, i64, String, String, String, Option<String>, Option<String>)> = sqlx::query_as(
+    type SlotRow = (
+        i64,
+        i64,
+        Option<i64>,
+        i64,
+        String,
+        String,
+        String,
+        Option<String>,
+        Option<String>,
+    );
+    let slot_rows: Vec<SlotRow> = sqlx::query_as(
         "SELECT s.id, s.session_id, s.room_id, s.tutor_id, s.start_time, s.end_time, s.status, s.code, r.layout_json
          FROM slots s
          LEFT JOIN rooms r ON s.room_id = r.id

backend/src/routes/auth_routes.rs

@@ -111,6 +111,15 @@ async fn refresh(
 
     let claims = auth::decode_jwt(&refresh_token, &state.jwt_secret, true)?;
 
+    // Re-check is_active so deactivated tutors cannot refresh their session
+    let is_active: Option<bool> = sqlx::query_scalar("SELECT is_active FROM tutors WHERE id = ?")
+        .bind(claims.sub)
+        .fetch_optional(&state.pool)
+        .await?;
+    if !is_active.unwrap_or(false) {
+        return Err(AppError::Unauthorized);
+    }
+
     // Issue new access token
     let access_token =
         auth::encode_jwt(claims.sub, claims.is_superadmin, &state.jwt_secret, false)?;
@@ -177,10 +186,11 @@ mod tests {
     #[sqlx::test(migrations = "./migrations")]
     async fn login_returns_superadmin_and_cookies(pool: sqlx::SqlitePool) {
         let hash = bcrypt::hash("secret", 4).unwrap();
-        sqlx::query("INSERT INTO tutors (name,email,password_hash) VALUES (?,?,?)")
+        sqlx::query("INSERT INTO tutors (name,email,password_hash,is_active) VALUES (?,?,?,?)")
             .bind("Test")
             .bind("t@test.com")
             .bind(&hash)
+            .bind(true)
             .execute(&pool)
             .await
             .unwrap();

backend/src/routes/courses.rs

@@ -66,11 +66,13 @@ async fn assign_tutor(
         return Err(AppError::Unauthorized);
     }
 
-    // Verify tutor is active
-    let is_active: bool = sqlx::query_scalar("SELECT is_active FROM tutors WHERE id = ?")
-        .bind(req.tutor_id)
-        .fetch_one(&pool)
-        .await?;
+    // Verify tutor exists and is active
+    let maybe_active: Option<bool> =
+        sqlx::query_scalar("SELECT is_active FROM tutors WHERE id = ?")
+            .bind(req.tutor_id)
+            .fetch_optional(&pool)
+            .await?;
+    let is_active = maybe_active.ok_or(AppError::NotFound)?;
     if !is_active {
         return Err(AppError::BadRequest(
             "Tutor:in ist inaktiv und kann nicht zugewiesen werden.".into(),

backend/src/routes/rooms.rs

@@ -57,11 +57,12 @@ fn validate_layout(layout: &[LayoutElement]) -> Result<(), AppError> {
                 "element outside of 100x100 canvas".into(),
             ));
         }
-        // 0.5-step multiple check
-        if (elem.x * 2.0).fract() != 0.0
-            || (elem.y * 2.0).fract() != 0.0
-            || (elem.width * 2.0).fract() != 0.0
-            || (elem.height * 2.0).fract() != 0.0
+        // 0.5-step multiple check — use epsilon comparison to avoid IEEE-754 fract() edge cases
+        let not_half_step = |v: f64| ((v * 2.0).round() - v * 2.0).abs() > f64::EPSILON;
+        if not_half_step(elem.x)
+            || not_half_step(elem.y)
+            || not_half_step(elem.width)
+            || not_half_step(elem.height)
         {
             return Err(AppError::BadRequest(
                 "coords/dims must be multiples of 0.5".into(),
@@ -176,11 +177,15 @@ async fn delete_room(
         )));
     }
 
-    sqlx::query("DELETE FROM rooms WHERE id = ?")
+    let result = sqlx::query("DELETE FROM rooms WHERE id = ?")
         .bind(id)
         .execute(&pool)
         .await?;
 
+    if result.rows_affected() == 0 {
+        return Err(AppError::NotFound);
+    }
+
     Ok(StatusCode::NO_CONTENT)
 }
 

backend/src/routes/tutors.rs

@@ -114,11 +114,13 @@ async fn delete_tutor(
         return Err(AppError::Conflict("cannot delete yourself".into()));
     }
 
-    // Check for references
+    // Wrap reference checks and DELETE in one transaction to avoid TOCTOU
+    let mut tx = pool.begin().await?;
+
     let course_count: i64 =
         sqlx::query_scalar("SELECT COUNT(*) FROM tutor_courses WHERE tutor_id = ?")
             .bind(id)
-            .fetch_one(&pool)
+            .fetch_one(&mut *tx)
             .await?;
     if course_count > 0 {
         return Err(AppError::Conflict(format!(
@@ -129,7 +131,7 @@ async fn delete_tutor(
 
     let slot_count: i64 = sqlx::query_scalar("SELECT COUNT(*) FROM slots WHERE tutor_id = ?")
         .bind(id)
-        .fetch_one(&pool)
+        .fetch_one(&mut *tx)
         .await?;
     if slot_count > 0 {
         return Err(AppError::Conflict(format!(
@@ -140,7 +142,7 @@ async fn delete_tutor(
 
     let note_count: i64 = sqlx::query_scalar("SELECT COUNT(*) FROM notes WHERE tutor_id = ?")
         .bind(id)
-        .fetch_one(&pool)
+        .fetch_one(&mut *tx)
         .await?;
     if note_count > 0 {
         return Err(AppError::Conflict(format!(
@@ -151,7 +153,7 @@ async fn delete_tutor(
 
     match sqlx::query("DELETE FROM tutors WHERE id = ?")
         .bind(id)
-        .execute(&pool)
+        .execute(&mut *tx)
         .await
     {
         Ok(_) => {}
@@ -163,6 +165,7 @@ async fn delete_tutor(
         Err(e) => return Err(AppError::Db(e)),
     }
 
+    tx.commit().await?;
     Ok(StatusCode::NO_CONTENT)
 }
 pub fn router() -> Router<AppState> {

frontend/src/lib/RoomCanvas.svelte

@@ -6,7 +6,7 @@
         editable?: boolean;
         clickable?: boolean;
         snapStep?: number;
-        onElementClick?: (el: LayoutElement) => void;
+        onElementClick?: (el: LayoutElement | null) => void;
         onLayoutChange?: (elements: LayoutElement[]) => void;
         selectedId?: string | null;
         occupiedSeatIds?: string[];
@@ -30,23 +30,26 @@
     let draggingId = $state<string | null>(null);
     let resizingId = $state<string | null>(null);
     let resizeDir = $state<'e' | 's' | 'se' | null>(null);
-    
+
     let startX = 0;
     let startY = 0;
     let initialX = 0;
     let initialY = 0;
     let initialW = 0;
     let initialH = 0;
+    let dragMoved = false;
 
     const GRID_SIZE = 40;
 
     function handleMouseDown(e: MouseEvent, el: LayoutElement) {
-        if (editable || clickable) {
+        if (clickable && !editable) {
             onElementClick?.(el);
+            return;
         }
         if (!editable) return;
 
         draggingId = el.id;
+        dragMoved = false;
         startX = e.clientX;
         startY = e.clientY;
         initialX = el.x;
@@ -69,6 +72,7 @@
         if (!editable) return;
 
         if (draggingId) {
+            dragMoved = true;
             const index = elements.findIndex(el => el.id === draggingId);
             if (index !== -1) {
                 const dx = (e.clientX - startX) / GRID_SIZE;
@@ -117,12 +121,22 @@
     }
 
     function handleWindowMouseUp() {
-        if ((draggingId || resizingId) && editable) {
-            onLayoutChange?.(elements);
+        if (editable) {
+            if (draggingId) {
+                if (dragMoved) {
+                    onLayoutChange?.(elements);
+                } else {
+                    const el = elements.find(e => e.id === draggingId);
+                    if (el) onElementClick?.(el);
+                }
+            } else if (resizingId) {
+                onLayoutChange?.(elements);
+            }
         }
         draggingId = null;
         resizingId = null;
         resizeDir = null;
+        dragMoved = false;
     }
 
     // Compute viewBox based on elements or default
@@ -147,12 +161,12 @@
     onclick={(e) => {
         // Deselect if clicking on empty canvas
         if (editable && e.target === e.currentTarget) {
-            onElementClick?.(null as unknown as LayoutElement);
+            onElementClick?.(null);
         }
     }}
 >
     <!-- Grid -->
-    {#if editable}
+    {#if editable && snapStep > 0}
         <defs>
             <pattern id="grid" width={GRID_SIZE * snapStep} height={GRID_SIZE * snapStep} patternUnits="userSpaceOnUse">
                 <path d="M {GRID_SIZE * snapStep} 0 L 0 0 0 {GRID_SIZE * snapStep}" fill="none" stroke="var(--rule-soft)" stroke-width="1" stroke-dasharray="2,2"/>

frontend/src/routes/admin/live/[slotId]/+page.svelte

@@ -110,7 +110,7 @@
   const absentCount = $derived(students.length - presentCount);
   
   const occupiedSeatIds = $derived(attendances.map(a => a.seat_id).filter(Boolean) as string[]);
-  const studentNamesBySeat = $derived(() => {
+  const studentNamesBySeat = $derived((() => {
       const map: Record<string, string> = {};
       for (const att of attendances) {
           if (att.seat_id) {
@@ -121,7 +121,7 @@
           }
       }
       return map;
-  });
+  })());
 
   function handleSeatClick(el: { id: string } | null) {
       if (!el) {
@@ -198,7 +198,7 @@
             elements={slot.layout ?? []} 
             clickable={true} 
             occupiedSeatIds={occupiedSeatIds} 
-            studentNames={studentNamesBySeat()} 
+            studentNames={studentNamesBySeat}
             onElementClick={handleSeatClick} 
           />
         </div>

frontend/src/routes/admin/rooms/+page.svelte

@@ -35,11 +35,12 @@
 
   async function deleteRoom(id: number) {
     if (!confirm('Raum wirklich löschen?')) return;
+    errorMsg = null;
     try {
       await api.admin.rooms.delete(id);
       rooms = await api.admin.rooms.list();
     } catch (err) {
-      if (err instanceof Error) alert(err.message);
+      errorMsg = err instanceof Error ? err.message : 'Raum konnte nicht gelöscht werden.';
     }
   }
 </script>

frontend/src/routes/admin/rooms/[roomId]/+page.svelte

@@ -33,7 +33,7 @@
 
   function addElement(type: LayoutElement['type']) {
     if (!room) return;
-    const id = Math.random().toString(36).substr(2, 9);
+    const id = Math.random().toString(36).slice(2, 11);
     let nextLabel: string;
     
     if (type === 'seat') {
@@ -61,7 +61,7 @@
 
   function duplicateElement() {
     if (!room || !selectedElement) return;
-    const id = Math.random().toString(36).substr(2, 9);
+    const id = Math.random().toString(36).slice(2, 11);
     
     let nextLabel: string;
     if (selectedElement.type === 'seat') {

frontend/src/routes/s/[code]/+page.svelte

@@ -80,6 +80,7 @@
 
   async function checkin(seatId?: string) {
     if (!selectedStudent) return;
+    errorMsg = '';
     try {
       await api.checkin.post(code, selectedStudent.id, seatId);
     } catch (e) {
@@ -219,7 +220,7 @@
             elements={layout}
             clickable={true}
             occupiedSeatIds={occupiedSeatIds}
-            onElementClick={(el) => { if (el && el.type === 'seat') checkin(el.id); }}
+            onElementClick={(el) => { if (el?.type === 'seat' && !occupiedSeatIds.includes(el.id)) checkin(el.id); }}
           />
         </div>
 
@@ -254,9 +255,10 @@
         </div>
 
         <div style="overflow-x:auto">
-          <RoomCanvas 
-            elements={layout} 
-            mySeatId={myAttendance?.seat_id ?? null} 
+          <RoomCanvas
+            elements={layout}
+            mySeatId={myAttendance?.seat_id ?? null}
+            occupiedSeatIds={occupiedSeatIds}
           />
         </div>
 
@@ -329,7 +331,7 @@
             elements={layout}
             clickable={true}
             occupiedSeatIds={occupiedSeatIds}
-            onElementClick={(el) => { if (el && el.type === 'seat') checkin(el.id); }}
+            onElementClick={(el) => { if (el?.type === 'seat' && !occupiedSeatIds.includes(el.id)) checkin(el.id); }}
           />
 
           <div style="display:flex;gap:20px" class="tiny">
@@ -378,9 +380,10 @@
             </div>
           </div>
 
-          <RoomCanvas 
-            elements={layout} 
-            mySeatId={myAttendance?.seat_id ?? null} 
+          <RoomCanvas
+            elements={layout}
+            mySeatId={myAttendance?.seat_id ?? null}
+            occupiedSeatIds={occupiedSeatIds}
           />
 
           <div style="display:flex;gap:10px">