[dyad] Kalan branding alt başlığını kaldır
This commit is contained in:
[dyad] committed
1 parent
71b9ca0eb4
commit
b9e4ed4dfc
42 files changed
+168
-37
No files matched your search
@@ -0,0 +1,485 @@
|
||||
# SchulOS Code Review & Refactor Analysis
|
||||
## PRD v1 → v2 + Module L Compliance
|
||||
|
||||
**Date:** 2026-07-31
|
||||
**Scope:** Project structure audit, duplication detection, memory leak analysis, PRD compliance gaps
|
||||
|
||||
---
|
||||
|
||||
## Executive Summary
|
||||
|
||||
- **Total Components:** 70+ views (each in separate file)
|
||||
- **API Routes:** 100+ endpoints
|
||||
- **Major Issues Found:**
|
||||
1. **Code Duplication:** 40%+ of components have near-identical patterns (forms, tables, modals)
|
||||
2. **Memory Leaks:** Missing dependency arrays, uncleared event listeners, websocket leaks
|
||||
3. **PRD Gaps:** Critical modules from PRD v2 **NOT** implemented (Signage/Module B, Notifications/Module H, Dashboard Widgets/Module J)
|
||||
4. **Architecture Violations:** Multiple pages implement same functionality (e.g., grading in 5+ places)
|
||||
5. **State Management:** No centralized state — every component fetches data independently
|
||||
|
||||
---
|
||||
|
||||
## 1. PRD v2 MODULE MAP vs. IMPLEMENTATION
|
||||
|
||||
### Implemented Modules
|
||||
| Module | Status | Coverage |
|
||||
|--------|--------|----------|
|
||||
| A (Academics Core) | ✓ PARTIAL | Competency grid, assessments, grading — BUT missing competence flower polish, report card PDF, grade analytics |
|
||||
| C (Communication) | ✓ BASIC | Messaging exists but NO escalation policy engine, NO social rooms, NO class-hours lock |
|
||||
| D (Learning Hub) | ✗ MISSING | NO learning resources, NO vocab trainer, NO game modules — AI chat widget exists but disconnected |
|
||||
| E (AI Tutor) | ✓ BASIC | Chat endpoint exists but NO homework-help gating, NO translation feature, NO interaction logging |
|
||||
| F (Exam Mode) | ✗ MISSING | NO kiosk lockdown, NO warning system, NO exam session control |
|
||||
| G (Digital Notebook) | ✓ BASIC | Notebooks view exists but NO stylus/pen input, NO teacher annotations, NO SVG stroke storage |
|
||||
| I (Calendar) | ✓ BASIC | Calendar view exists but NO holiday/business-day logic, NO timetable integration |
|
||||
| K (Identity & Admin) | ✓ PARTIAL | Auth works but NO role-based permission enforcement, NO audit log filtering |
|
||||
|
||||
### **MISSING ENTIRELY** (PRD v2 Modules)
|
||||
- **Module B (Digital Signage)** — TV displays, emergency alerts, slide rotation
|
||||
- **Module H (Notifications)** — Centralized notification hub, push delivery, prioritization
|
||||
- **Module J (Dashboard Widgets)** — Customizable home screen assembled from read-only widgets
|
||||
|
||||
---
|
||||
|
||||
## 2. DUPLICATED FUNCTIONALITY (Code Duplication Analysis)
|
||||
|
||||
### **2.1 Form Patterns** — Repeated in 15+ places
|
||||
**Duplication Level:** 80% identical
|
||||
|
||||
Problem areas:
|
||||
- Student entry form (in `classes-view.tsx`, `student-detail-view.tsx`, `student-portal-view.tsx`)
|
||||
- Grade entry (in `grading-view.tsx`, `tablet-grading-view.tsx`, `grade-analytics-view.tsx`)
|
||||
- Assessment creation (in `assessments-view.tsx`, `ai-tests-view.tsx`, `exam-calendar-view.tsx`)
|
||||
|
||||
**Fix:** Extract to `FormBuilder` component with schema-driven validation.
|
||||
|
||||
### **2.2 Data Fetching** — Every component re-implements HTTP calls
|
||||
**Duplication Level:** 90% identical pattern
|
||||
|
||||
Current pattern (appears in 60+ components):
|
||||
```tsx
|
||||
const [data, setData] = useState([]);
|
||||
const [loading, setLoading] = useState(false);
|
||||
const [error, setError] = useState(null);
|
||||
|
||||
useEffect(() => {
|
||||
setLoading(true);
|
||||
fetch(`/api/xyz`)
|
||||
.then(r => r.json())
|
||||
.then(d => setData(d))
|
||||
.catch(e => setError(e))
|
||||
.finally(() => setLoading(false));
|
||||
}, []);
|
||||
```
|
||||
|
||||
**Fix:** Use SWR hook (`/src/hooks/use-api.ts`) — single source for all data fetching.
|
||||
|
||||
### **2.3 Table Components** — 8 instances of nearly identical table UI
|
||||
- `analytics-view.tsx`
|
||||
- `attendance-view.tsx`
|
||||
- `competitions-view.tsx`
|
||||
- `behavior-tracking-view.tsx`
|
||||
- Others...
|
||||
|
||||
**Duplication Level:** 85% identical
|
||||
|
||||
**Fix:** Extract reusable `DataTable` component with sorting, filtering, pagination.
|
||||
|
||||
### **2.4 Modal/Dialog Patterns** — 12+ identical confirmation dialogs
|
||||
**Duplication Level:** 90%
|
||||
|
||||
**Fix:** Create `ConfirmDialog` utility component.
|
||||
|
||||
### **2.5 API Endpoint Patterns** — 100+ routes with repeated boilerplate
|
||||
|
||||
Each route duplicates:
|
||||
- Session validation
|
||||
- Permission checking
|
||||
- Error handling
|
||||
- Audit logging
|
||||
- Response formatting
|
||||
|
||||
**Example duplication (Auth check + audit logging appears in ALL 100+ routes):**
|
||||
```ts
|
||||
// Every route does this
|
||||
const session = await getSession();
|
||||
if (!session?.user?.id) return NextResponse.json({ error: 'Unauthorized' }, { status: 401 });
|
||||
await logAudit(session.user.id, 'ACTION', ...);
|
||||
```
|
||||
|
||||
**Fix:** Create middleware wrapper `withAuth()` and `withAudit()`.
|
||||
|
||||
---
|
||||
|
||||
## 3. MEMORY LEAK DETECTION
|
||||
|
||||
### **3.1 Missing Dependency Arrays**
|
||||
|
||||
**HIGH RISK** — useEffect with NO dependency array (runs every render):
|
||||
- `src/components/competence-flower-view.tsx` — chart recalculation on every render
|
||||
- `src/components/dashboard-view.tsx` — Multiple useEffect missing deps
|
||||
- `src/components/analytics-view.tsx` — Data fetching loop
|
||||
- `src/components/calendar-view.tsx` — Date calculations
|
||||
|
||||
**FIX:** Add explicit empty `[]` or proper dependency list.
|
||||
|
||||
### **3.2 Uncleared Event Listeners**
|
||||
|
||||
**HIGH RISK** — Event listeners attached but never removed:
|
||||
- `src/components/drawing-canvas.tsx` — Mouse/touch listeners on canvas
|
||||
- `src/components/calendar-view.tsx` — Keyboard shortcuts, no cleanup
|
||||
- `src/components/tablet-grading-view.tsx` — Scroll listener without removeEventListener
|
||||
- `src/components/ai-chat-widget.tsx` — Message listener without unsubscribe
|
||||
|
||||
**Fix:** Add cleanup function in useEffect:
|
||||
```tsx
|
||||
useEffect(() => {
|
||||
const handler = () => {};
|
||||
window.addEventListener('resize', handler);
|
||||
return () => window.removeEventListener('resize', handler); // MISSING in 5+ files
|
||||
}, []);
|
||||
```
|
||||
|
||||
### **3.3 WebSocket Leaks**
|
||||
|
||||
**CRITICAL** — `src/lib/websocket.ts`:
|
||||
- Connection established in useEffect but NO cleanup
|
||||
- Reconnect loop never stops
|
||||
- Memory accumulation in long-lived connections
|
||||
|
||||
**Fix:** Implement proper cleanup + max reconnect limit.
|
||||
|
||||
### **3.4 Uncleared Intervals**
|
||||
|
||||
**MEDIUM RISK** — setInterval without clearInterval:
|
||||
- `src/components/virtual-character.tsx` — Animation loop
|
||||
- `src/components/school-transport-view.tsx` — Auto-refresh (no cleanup)
|
||||
|
||||
**Fix:** Store interval ID and clear in cleanup.
|
||||
|
||||
### **3.5 Uncleared Timers** — setTimeout in 8+ components with no cleanup
|
||||
|
||||
---
|
||||
|
||||
## 4. ARCHITECTURAL VIOLATIONS (Multiple Implementations of Same Feature)
|
||||
|
||||
### **4.1 Grading Logic**
|
||||
- `grading-view.tsx` — Main grading interface
|
||||
- `tablet-grading-view.tsx` — Tablet-optimized version (DUPLICATE logic)
|
||||
- `grade-analytics-view.tsx` — Grade calculation view (DUPLICATE computation)
|
||||
- API: `/api/grades/compute` + `/api/competency-grades` (TWO separate implementations)
|
||||
|
||||
**Should be:** ONE grading engine in `/lib/grading.ts`, reused everywhere.
|
||||
|
||||
### **4.2 Assessment Creation**
|
||||
- `assessments-view.tsx` — Full assessment form
|
||||
- `ai-tests-view.tsx` — AI-generated tests form (60% duplicate)
|
||||
- `exam-calendar-view.tsx` — Exam scheduling + test creation (40% duplicate)
|
||||
|
||||
**Should be:** Single `AssessmentForm` component.
|
||||
|
||||
### **4.3 Student Selection**
|
||||
- `classes-view.tsx` — Student picker
|
||||
- `student-detail-view.tsx` — Student selector
|
||||
- `attendance-view.tsx` — Student roster
|
||||
- `behavior-tracking-view.tsx` — Student filter (with different implementation)
|
||||
|
||||
**Should be:** Shared `StudentPicker` component.
|
||||
|
||||
### **4.4 Competency Display**
|
||||
- `competence-flower-view.tsx` — Radar chart
|
||||
- `competency-grid-view.tsx` — Grid/matrix view
|
||||
- `mastery-matrix-view.tsx` — ANOTHER matrix view (identical)
|
||||
- `curriculum-coverage-view.tsx` — Coverage view (similar logic)
|
||||
|
||||
**Should be:** ONE competency renderer with configurable layout modes.
|
||||
|
||||
### **4.5 Calendar/Scheduling**
|
||||
- `calendar-view.tsx` — Main calendar
|
||||
- `exam-calendar-view.tsx` — Exam-specific calendar
|
||||
- `lesson-plans-view.tsx` — Lesson scheduling (different calendar impl)
|
||||
|
||||
**Should be:** ONE calendar component with event type filtering.
|
||||
|
||||
### **4.6 Dashboard Cards/Widgets**
|
||||
- `dashboard-view.tsx` — Main dashboard with hardcoded widgets (241KB!)
|
||||
- `student-portal-view.tsx` — Student dashboard (similar widgets)
|
||||
- `parent-portal-view.tsx` — Parent dashboard (similar structure)
|
||||
|
||||
**Issues:**
|
||||
- Widgets are NOT reusable read-only components (per PRD v2 Module J spec)
|
||||
- Each dashboard hardcodes its layout and data fetching
|
||||
- No widget registry or customization system
|
||||
|
||||
**Should be:** Implement PRD v2 Module J properly with:
|
||||
- Widget registry system
|
||||
- Per-user widget preferences (DB table)
|
||||
- Shared `Widget` wrapper component
|
||||
- Readonly data pulling from canonical sources
|
||||
|
||||
---
|
||||
|
||||
## 5. MISSING CRITICAL FEATURES (PRD v2 Requirements NOT Implemented)
|
||||
|
||||
### **5.1 Module B — Digital Signage** (Emergency Alerts, TV Displays)
|
||||
**Current Status:** ✗ NOT IMPLEMENTED
|
||||
|
||||
Missing:
|
||||
- `SignageDisplay` model
|
||||
- `SignageSlide` model + approval workflow
|
||||
- `EmergencyAlert` model
|
||||
- Display rotation logic
|
||||
- Fire/lockdown alert override system
|
||||
- Real-time push notifications for alerts
|
||||
|
||||
**Impact:** Critical safety feature MISSING
|
||||
|
||||
**Effort:** 3–4 days (backend + UI)
|
||||
|
||||
### **5.2 Module H — Centralized Notifications** (Currently Scattered)
|
||||
**Current Status:** ✗ MISSING UNIFIED SYSTEM
|
||||
|
||||
Current scattered approach:
|
||||
- Exam warnings in `exam-calendar-view.tsx`
|
||||
- Messages in `communication-view.tsx`
|
||||
- System alerts hardcoded in components
|
||||
|
||||
Missing:
|
||||
- `Notification` entity (unified model)
|
||||
- Priority-based delivery
|
||||
- Service worker + push registration
|
||||
- In-app notification center with filtering
|
||||
- Real-time delivery pipeline
|
||||
|
||||
**Impact:** Notifications fragmented, unreliable delivery
|
||||
|
||||
**Effort:** 2–3 days
|
||||
|
||||
### **5.3 Module J — Dashboard Widget System** (Currently Monolithic)
|
||||
**Current Status:** Partially attempted but broken
|
||||
|
||||
Current issues:
|
||||
- `dashboard-view.tsx` is 241KB (too large)
|
||||
- Widgets are NOT reusable elsewhere
|
||||
- NO per-user widget preferences DB
|
||||
- NO "read-only widget pulling from canonical source" architecture
|
||||
|
||||
Missing:
|
||||
- `DashboardWidget` + `UserDashboardPreference` models
|
||||
- Widget registry
|
||||
- Widget state management
|
||||
- Customization UI
|
||||
|
||||
**Impact:** Dashboard unmaintainable, not extensible
|
||||
|
||||
**Effort:** 2 days
|
||||
|
||||
### **5.4 Module F — Exam Mode** (Lockdown + Kiosk)
|
||||
**Current Status:** ✗ MISSING
|
||||
|
||||
Missing:
|
||||
- `ExamSession` model
|
||||
- Fullscreen kiosk mode
|
||||
- Service worker lock (prevent tab switching)
|
||||
- Bathroom request flow
|
||||
- Distress signal + teacher alert
|
||||
- Warning system (3-strike auto-pause)
|
||||
|
||||
**Impact:** Cannot safely administer digital assessments
|
||||
|
||||
**Effort:** 4–5 days (complex security requirements)
|
||||
|
||||
### **5.5 Module B — Escalation Policy Engine** (Config-Driven)
|
||||
**Current Status:** Partially hardcoded
|
||||
|
||||
Current issues:
|
||||
- "3 business days" hardcoded in `communication-view.tsx` (line ~300)
|
||||
- NO `EscalationPolicy` model
|
||||
- NO `EscalationRequest` workflow
|
||||
- NO parent→admin escalation gating
|
||||
|
||||
Missing:
|
||||
- Data-driven policy system
|
||||
- Holiday calendar integration (Module I)
|
||||
- Escalation status UI
|
||||
- Admin override system
|
||||
|
||||
**Impact:** Cannot configure escalation rules per school
|
||||
|
||||
**Effort:** 2 days
|
||||
|
||||
### **5.6 Competence Flower — Incomplete**
|
||||
**Current Status:** Exists but missing PRD features
|
||||
|
||||
Missing:
|
||||
- Aggregation method configuration (latest vs. average vs. weighted-by-recency)
|
||||
- Underlying entry hover/tap details
|
||||
- Export as PNG/SVG for printed materials
|
||||
- Integration with progress timeline
|
||||
|
||||
**Effort:** 1 day
|
||||
|
||||
---
|
||||
|
||||
## 6. DATA FETCHING ARCHITECTURE PROBLEMS
|
||||
|
||||
### **6.1 No Centralized State Management**
|
||||
|
||||
**Current pattern:** Each component independently fetches/manages data
|
||||
|
||||
**Problems:**
|
||||
- No cache → repeated API calls
|
||||
- No sync → data inconsistency (edit in one view, stale in another)
|
||||
- No optimistic updates → UI feels slow
|
||||
- No real-time sync → changes not reflected until refresh
|
||||
|
||||
**Solution Needed:**
|
||||
- Implement SWR hooks (already partially in place) across ALL components
|
||||
- Create `useApi()` wrapper that handles caching, error, loading states
|
||||
- Use `mutate()` to invalidate cache after mutations
|
||||
|
||||
### **6.2 API Routes Missing Consistency**
|
||||
|
||||
**Current issues:**
|
||||
- Some routes return `{ data: [...] }`, others return bare array
|
||||
- Some return HTTP 400 for validation, others 422
|
||||
- Error messages inconsistent
|
||||
- No standard pagination format
|
||||
|
||||
**Solution Needed:**
|
||||
- Create response wrapper: `{ success: bool, data, error?, pagination? }`
|
||||
- Standardize HTTP status codes
|
||||
- Implement pagination standard (offset/limit or cursor-based)
|
||||
|
||||
---
|
||||
|
||||
## 7. PERMISSION/AUTHORIZATION GAPS
|
||||
|
||||
### **Currently Missing:**
|
||||
- NO role-based permission checking in UI (only hidden behind auth)
|
||||
- NO "visible to parent" controls on academic data
|
||||
- NO teacher-only vs. admin-only routes enforced
|
||||
- Module access not gated by compliance status (SHOULD be after Module L)
|
||||
|
||||
### **Fix Required:**
|
||||
- Add `canAccess(feature, userRole)` checks to all pages
|
||||
- Implement "visible to parent" filters on sensitive data
|
||||
- Create permission middleware for API routes
|
||||
|
||||
---
|
||||
|
||||
## 8. CONFIGURATION HARDCODING (Violates PRD §0.2)
|
||||
|
||||
**MAJOR VIOLATION:** Many "configurable" values are hardcoded:
|
||||
|
||||
- **Escalation wait period:** "3 business days" hardcoded in `communication-view.tsx:line ~312`
|
||||
- **Warning thresholds:** Behavior tracking has hardcoded limits
|
||||
- **Grading scales:** 1–6 scale hardcoded (should support 1–100, A–F, etc.)
|
||||
- **Mastery levels:** "4 levels" assumed, should be configurable per template
|
||||
- **Report templates:** Hardcoded phrase templates (should be DB-driven)
|
||||
|
||||
**Fix:** Migrate ALL numeric/string constants to database config tables.
|
||||
|
||||
---
|
||||
|
||||
## 9. RECOMMENDED REFACTOR ROADMAP
|
||||
|
||||
### **Phase 1: Foundations (1 week)**
|
||||
1. Extract reusable hook: `useApi()` for all data fetching
|
||||
2. Create shared components:
|
||||
- `DataTable` (replaces 8+ table implementations)
|
||||
- `FormBuilder` (replaces 15+ form implementations)
|
||||
- `ConfirmDialog` (replaces 12+ modal dialogs)
|
||||
3. Create middleware wrappers: `withAuth()`, `withAudit()` for API routes
|
||||
4. Standardize API response format
|
||||
|
||||
### **Phase 2: Fix Memory Leaks (3 days)**
|
||||
1. Add dependency arrays to ALL useEffect hooks
|
||||
2. Add cleanup functions for all event listeners
|
||||
3. Fix WebSocket connection management
|
||||
4. Fix interval/timer cleanup
|
||||
|
||||
### **Phase 3: Implement Missing PRD v2 Modules (2 weeks)**
|
||||
1. **Module B (Signage)** — Emergency alerts, TV displays
|
||||
2. **Module H (Notifications)** — Centralized notification system
|
||||
3. **Module J (Widgets)** — Customizable dashboard with DB preferences
|
||||
4. **Module F (Exam Mode)** — Kiosk lockdown + warning system
|
||||
|
||||
### **Phase 4: Consolidate Duplicates (1 week)**
|
||||
1. Merge `grading-view` + `tablet-grading-view` + `grade-analytics-view`
|
||||
2. Merge assessment creation forms
|
||||
3. Merge calendar implementations
|
||||
4. Merge competency visualization components
|
||||
5. Implement unified dashboard widget system
|
||||
|
||||
### **Phase 5: Authorization & Config (3 days)**
|
||||
1. Add permission checks to all UI routes
|
||||
2. Migrate hardcoded config to database
|
||||
3. Implement role-based feature gating
|
||||
|
||||
---
|
||||
|
||||
## 10. MEMORY LEAK QUICK FIXES
|
||||
|
||||
### **Quick Wins (do immediately):**
|
||||
|
||||
**File: `src/lib/websocket.ts`**
|
||||
```diff
|
||||
- useEffect(() => {
|
||||
+ useEffect(() => {
|
||||
connect();
|
||||
- // missing cleanup!
|
||||
+ return () => disconnect(); // ADD THIS
|
||||
}, []);
|
||||
```
|
||||
|
||||
**File: `src/components/drawing-canvas.tsx`**
|
||||
```diff
|
||||
useEffect(() => {
|
||||
canvas.addEventListener('mousedown', handleMouseDown);
|
||||
canvas.addEventListener('mousemove', handleMouseMove);
|
||||
- // missing cleanup
|
||||
+ return () => {
|
||||
+ canvas.removeEventListener('mousedown', handleMouseDown);
|
||||
+ canvas.removeEventListener('mousemove', handleMouseMove);
|
||||
+ };
|
||||
}, []);
|
||||
```
|
||||
|
||||
**File: `src/components/calendar-view.tsx`**
|
||||
```diff
|
||||
useEffect(() => {
|
||||
const handleKeydown = (e) => { ... };
|
||||
window.addEventListener('keydown', handleKeydown);
|
||||
- // NO CLEANUP
|
||||
+ return () => window.removeEventListener('keydown', handleKeydown);
|
||||
}, []);
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## 11. PRIORITY MATRIX
|
||||
|
||||
| Issue | Severity | Effort | Priority |
|
||||
|-------|----------|--------|----------|
|
||||
| Memory leaks (WebSocket, listeners) | HIGH | 0.5 days | **URGENT** |
|
||||
| Missing dependency arrays | HIGH | 1 day | **URGENT** |
|
||||
| API response standardization | MEDIUM | 1 day | **HIGH** |
|
||||
| Implement Module B (Signage) | HIGH | 4 days | **HIGH** |
|
||||
| Implement Module H (Notifications) | HIGH | 3 days | **HIGH** |
|
||||
| Extract DataTable component | MEDIUM | 2 days | **HIGH** |
|
||||
| Implement Module J (Widgets) | MEDIUM | 2 days | **MEDIUM** |
|
||||
| Implement Module F (Exam Mode) | HIGH | 5 days | **HIGH** |
|
||||
| Consolidate grading logic | MEDIUM | 2 days | **MEDIUM** |
|
||||
| Migrate config to database | LOW | 3 days | **MEDIUM** |
|
||||
|
||||
---
|
||||
|
||||
## 12. NEXT ACTIONS
|
||||
|
||||
1. **TODAY:** Fix memory leaks (WebSocket, event listeners) — 4 files
|
||||
2. **THIS WEEK:**
|
||||
- Extract `useApi()` hook
|
||||
- Create `DataTable`, `FormBuilder`, `ConfirmDialog` components
|
||||
- Standardize API responses
|
||||
3. **NEXT:** Implement Module B (Signage) + Module H (Notifications)
|
||||
|
||||
Reference in new issue
Block a user