301 lines
21 KiB
Markdown
301 lines
21 KiB
Markdown
# Phase 1 - Architecture Audit
|
|
|
|
## Executive Summary
|
|
|
|
ภาพรวมสถาปัตยกรรมของระบบปัจจุบันมีแกนหลักที่ชัดเจนในระดับ app-owned stack คือ `Next.js App Router + Auth.js + Drizzle + PostgreSQL + TanStack Query` และหลาย feature หลักเดินตาม pattern `page -> feature api/service -> route handler -> server data -> db` ได้ดี โดยเฉพาะ `training-records`, `courses`, `announcements`, `reports`, และ `permission-management`
|
|
|
|
อย่างไรก็ตาม implementation ปัจจุบันยังมี architectural drift อยู่หลายจุดเมื่อเทียบกับ target architecture ในเอกสารโครงการ โดยประเด็นสำคัญที่สุดคือ runtime ยังพึ่งพาโมเดล `employees` อย่างกว้างขวาง แม้เอกสารสถาปัตยกรรมจะระบุว่า `users` ควรเป็น runtime person entity หลักและ `employees` เป็น legacy migration data เท่านั้น นอกจากนี้ยังพบ production dashboard surface ที่ยังรวม route กลุ่ม demo/template/legacy และมีความไม่สม่ำเสมอของ guard placement และ route contract บางจุด
|
|
|
|
## System Shape Observed
|
|
|
|
- Framework: Next.js 16 App Router
|
|
- Auth boundary: `src/auth.ts`, `src/lib/auth/session.ts`, `src/lib/auth/page-guards.ts`, `src/proxy.ts`
|
|
- Data boundary: `src/app/api/*` route handlers + Drizzle schema ใน `src/db/schema.ts`
|
|
- Feature modularization: ส่วนใหญ่ใช้ `src/features/<feature>` ตามแนวทางที่เอกสารกำหนด
|
|
- Canonical app-owned domains ที่เห็นชัด: `users`, `organizations`, `memberships`, `courses`, `training_records`, `training_matrices`, `training_policies`, `announcements`, `online_lessons`, `reports`, `permissions`
|
|
|
|
## Strengths
|
|
|
|
- มีการแยก auth/session, page guard, API handler, และ feature server helpers ค่อนข้างชัด
|
|
- route handlers สำคัญส่วนมากไม่เข้าฐานข้อมูลจาก UI โดยตรง แต่ผ่าน service/server data helpers
|
|
- RBAC model แบบ `systemRole + membership.role + effective permissions` ถูกวางโครงไว้ครบทั้ง session และ permission-resolution
|
|
- feature หลักจำนวนมากเดินตาม shared stack เดียวกัน เช่น `PageContainer`, TanStack Query, shared table/form primitives
|
|
|
|
## Findings
|
|
|
|
### [High] Runtime architecture ยังผูกกับ legacy `employees` อย่างลึก แม้ target architecture ระบุให้ `users` เป็น runtime person entity หลัก
|
|
|
|
**Module:** Auth, Schema, Training Records, Reports, Overview, Employee Directory, Training Policy
|
|
|
|
**ตำแหน่งที่พบ:**
|
|
|
|
- `src/db/schema.ts` ประมาณบรรทัด 81, 281, 438, 541
|
|
- `src/auth.ts` ประมาณบรรทัด 14, 126, 155, 285
|
|
- `src/features/employees/server/employee-identity-linking.ts`
|
|
- `src/features/training-records/server/training-record-data.ts`
|
|
- `src/features/reports/server/report-data.ts`
|
|
- `src/features/overview/server/overview-data.ts`
|
|
- `src/features/training-policy/server/employee-training-target-data.ts`
|
|
|
|
**รายละเอียด:**
|
|
|
|
เอกสาร `docs/PROJECT_ARCHITECTURE.md` และ `AGENTS.md` ระบุว่า runtime person data ควรอยู่บน `users` และ `employees` เป็น legacy migration data เท่านั้น แต่ implementation ปัจจุบันยังใช้ `employees` เป็น dependency หลักในหลาย workflow สำคัญ เช่น:
|
|
|
|
- `users.employeeId` ยังอ้างอิง `employees.id`
|
|
- มีตาราง `user_employee_maps`
|
|
- `training_records.employeeId` ยังอ้างอิง `employees.id`
|
|
- `employee_training_targets.employeeId` ยังถูกใช้ใน target resolution
|
|
- session enrichment ใน `src/auth.ts` เรียก `syncUserEmployeeLink(...)` ตอน login
|
|
- reports, overview, training matrix compliance, และ training records query หลักยัง join ผ่าน `employees`
|
|
|
|
**ผลกระทบ:**
|
|
|
|
- target architecture กับ implementation ปัจจุบันไม่สอดคล้องกัน
|
|
- การพัฒนาฟีเจอร์ใหม่มีความเสี่ยงสร้าง dual-source-of-truth ระหว่าง `users` และ `employees`
|
|
- การวาง roadmap ย้ายข้อมูลหรือ permission scoping ต่อจากนี้จะซับซ้อนขึ้น
|
|
|
|
**หลักฐานจากโค้ด:**
|
|
|
|
- `src/auth.ts` เรียก `syncUserEmployeeLink(...)` ระหว่าง authorize และเก็บ `employeeId` ลง session
|
|
- `src/db/schema.ts` ยังเก็บ foreign key หลายจุดไปที่ `employees`
|
|
- `src/features/reports/server/report-data.ts` และ `src/features/overview/server/overview-data.ts` ใช้ `employees` เป็นฐานการคำนวณ scope และ dataset
|
|
|
|
**สถานการณ์ตัวอย่าง:**
|
|
|
|
หากทีมพัฒนาฟีเจอร์ใหม่โดยอิงเอกสารอย่างเดียว จะเข้าใจว่า `users` คือ runtime source of truth แต่เมื่อเชื่อมกับ reports หรือ training records จริง ระบบยังต้องพึ่ง `employees` อยู่ ทำให้เกิดการ implement ผิดชั้นข้อมูลได้ง่าย
|
|
|
|
**คำแนะนำ:**
|
|
|
|
ระบุสถานะสถาปัตยกรรมปัจจุบันให้ชัดว่าอยู่ในช่วง transition และจัดทำ migration roadmap ระดับสถาปัตยกรรมก่อน coding phase แยกให้ชัดว่า domain ใดใช้ `users` จริงแล้ว และ domain ใดยังผูกกับ `employees`
|
|
|
|
**ต้องแก้ก่อน Production หรือไม่:** ใช่
|
|
|
|
### [Medium] Dashboard production surface ยังรวม route กลุ่ม demo/template/legacy ไว้ใน `app/dashboard`
|
|
|
|
**Module:** Dashboard routing, legacy/template features
|
|
|
|
**ตำแหน่งที่พบ:**
|
|
|
|
- `src/app/dashboard/billing/page.tsx`
|
|
- `src/app/dashboard/chat/page.tsx`
|
|
- `src/app/dashboard/forms/page.tsx`
|
|
- `src/app/dashboard/forms/advanced/page.tsx`
|
|
- `src/app/dashboard/forms/basic/page.tsx`
|
|
- `src/app/dashboard/forms/multi-step/page.tsx`
|
|
- `src/app/dashboard/forms/sheet-form/page.tsx`
|
|
- `src/app/dashboard/react-query/page.tsx`
|
|
- `src/app/dashboard/kanban/page.tsx`
|
|
- `src/app/dashboard/product/page.tsx`
|
|
- `src/app/dashboard/workspaces/page.tsx`
|
|
- `src/app/dashboard/elements/icons/page.tsx`
|
|
- `src/app/dashboard/exclusive/page.tsx`
|
|
|
|
**รายละเอียด:**
|
|
|
|
เอกสารสถาปัตยกรรมระบุให้บางพื้นที่ถูกมองเป็น legacy/template เท่านั้น แต่ route เหล่านี้ยังอยู่ใต้ dashboard จริง ทำให้ production route tree ปัจจุบันปะปนระหว่าง business modules กับ sample/demo/template modules
|
|
|
|
**ผลกระทบ:**
|
|
|
|
- เพิ่มความคลุมเครือว่าหน้าใดเป็น production-supported feature
|
|
- ทำให้การ audit permission, navigation, QA scope, และ release readiness ซับซ้อนขึ้น
|
|
- เพิ่ม maintenance overhead เพราะ route เหล่านี้ต้องถูกคำนึงถึงใน middleware, session, layout, และ regression surface
|
|
|
|
**หลักฐานจากโค้ด:**
|
|
|
|
- `src/app/dashboard/chat/page.tsx` render `ChatViewPage` ตรง
|
|
- `src/app/dashboard/react-query/page.tsx` ใช้ `features/react-query-demo`
|
|
- `src/app/dashboard/billing/page.tsx` ระบุชัดว่าเป็น placeholder หลังถอด Clerk billing ออกแล้ว
|
|
|
|
**สถานการณ์ตัวอย่าง:**
|
|
|
|
เมื่อทีมทำ full-system regression หรือ permission audit จะต้องตัดสินใจเองว่าหน้าเหล่านี้อยู่ในขอบเขต production หรือไม่ เพราะ route ยังอยู่ใน tree เดียวกับโมดูลธุรกิจหลัก
|
|
|
|
**คำแนะนำ:**
|
|
|
|
กำหนด policy ระดับสถาปัตยกรรมให้ชัดว่าหน้า demo/template ต้องย้ายออก, ปิด route, หรือ mark เป็น non-production module อย่างเป็นระบบ
|
|
|
|
**ต้องแก้ก่อน Production หรือไม่:** ควรพิจารณา
|
|
|
|
### [Medium] การวาง authorization guard ไม่สม่ำเสมอระหว่าง page routes
|
|
|
|
**Module:** Dashboard route access control
|
|
|
|
**ตำแหน่งที่พบ:**
|
|
|
|
- `src/app/dashboard/training-matrix/page.tsx`
|
|
- `src/app/dashboard/organizers/page.tsx`
|
|
- `src/app/dashboard/overview/page.tsx`
|
|
- `src/app/dashboard/layout.tsx`
|
|
- `src/lib/auth/page-guards.ts`
|
|
- `src/proxy.ts`
|
|
|
|
**รายละเอียด:**
|
|
|
|
บางหน้าใช้ shared page guards ตามมาตรฐาน (`requireEmployeeDashboardAccess`, `requireHRDDashboardAccess`, `requirePermissionDashboardAccess`) แต่บางหน้าใช้ custom auth/redirect เอง และบางหน้าพึ่ง proxy หรือ parent layout แทน โดยไม่มี local page guard ตรงตัว
|
|
|
|
ตัวอย่าง:
|
|
|
|
- `src/app/dashboard/training-matrix/page.tsx` ไม่เรียก page guard เลย
|
|
- `src/app/dashboard/organizers/page.tsx` ใช้ `auth()` + `isHRD(...)` + `redirect(...)` เอง แทน shared page guard
|
|
- `src/app/dashboard/overview/page.tsx` ไม่มี guard ตรงตัว โดย guard อยู่ที่ `src/app/dashboard/overview/layout.tsx`
|
|
- `src/app/dashboard/layout.tsx` ไม่มี auth check ในตัวเอง และอาศัย `src/proxy.ts` เป็น outer boundary
|
|
|
|
**ผลกระทบ:**
|
|
|
|
- access policy อ่านยากและตรวจสอบยาก
|
|
- เพิ่มโอกาสให้ route ใหม่ถูกสร้างโดยลืมวาง guard ในตำแหน่งที่เหมาะสม
|
|
- ลดความสม่ำเสมอของ architecture ที่เอกสารพยายามกำหนดไว้
|
|
|
|
**หลักฐานจากโค้ด:**
|
|
|
|
- `src/app/dashboard/training-matrix/page.tsx` parse search params และ render listing ทันที
|
|
- `src/app/dashboard/organizers/page.tsx` ไม่ใช้ helper จาก `src/lib/auth/page-guards.ts`
|
|
|
|
**สถานการณ์ตัวอย่าง:**
|
|
|
|
หากมีการ refactor proxy หรือย้าย layout/sub-layout ในอนาคต หน้าเหล่านี้อาจเปลี่ยนพฤติกรรมการป้องกันโดยไม่ได้ตั้งใจ เพราะ guard placement ไม่ได้อยู่รูปแบบเดียวกัน
|
|
|
|
**คำแนะนำ:**
|
|
|
|
กำหนด architecture rule เดียวสำหรับ dashboard pages ว่าต้องใช้ shared page guard ใน route entrypoint ทุกครั้ง หรือบันทึกข้อยกเว้นเป็น explicit convention
|
|
|
|
**ต้องแก้ก่อน Production หรือไม่:** ควรพิจารณา
|
|
|
|
### [Medium] Route contract ของ Training Matrix ไม่สมบูรณ์: มี CTA ไปยัง route ที่ไม่มีอยู่จริง
|
|
|
|
**Module:** Training Matrix dashboard route
|
|
|
|
**ตำแหน่งที่พบ:**
|
|
|
|
- `src/app/dashboard/training-matrix/page.tsx` ประมาณบรรทัด 28
|
|
- ไม่พบ `src/app/dashboard/training-matrix/new/page.tsx`
|
|
|
|
**รายละเอียด:**
|
|
|
|
หน้า `training-matrix` สร้างปุ่ม `Add Rule` ที่ลิงก์ไป `/dashboard/training-matrix/new` แต่จากโครงสร้างไฟล์ปัจจุบันไม่พบ route ปลายทางดังกล่าว
|
|
|
|
**ผลกระทบ:**
|
|
|
|
- route map ของ feature ไม่ครบตามที่ UI สื่อสาร
|
|
- เพิ่มสัญญาณว่าระบบยังมี implementation gap ระหว่าง page shell กับ feature workflow จริง
|
|
|
|
**หลักฐานจากโค้ด:**
|
|
|
|
- `src/app/dashboard/training-matrix/page.tsx` มี `href='/dashboard/training-matrix/new'`
|
|
- ตรวจไม่พบไฟล์ `src/app/dashboard/training-matrix/new/page.tsx`
|
|
|
|
**สถานการณ์ตัวอย่าง:**
|
|
|
|
ผู้ใช้ HRD/แอดมินกด `Add Rule` แล้วไปสู่ 404 หรือเส้นทางที่ไม่รองรับ ทำให้ workflow หลักของ matrix management ไม่ต่อเนื่อง
|
|
|
|
**คำแนะนำ:**
|
|
|
|
ระบุให้ชัดว่า feature นี้ตั้งใจใช้ modal/sheet แทน route create หรือมี route create จริงแต่ยังขาด implementation จากนั้นปรับ route contract ให้สอดคล้อง
|
|
|
|
**ต้องแก้ก่อน Production หรือไม่:** ใช่
|
|
|
|
### [Low] API perimeter protection ใน `proxy.ts` ใช้ allowlist แบบบางส่วน ทำให้ boundary strategy ไม่สม่ำเสมอ
|
|
|
|
**Module:** Middleware / API protection strategy
|
|
|
|
**ตำแหน่งที่พบ:**
|
|
|
|
- `src/proxy.ts`
|
|
- เปรียบเทียบกับ API modules ใต้ `src/app/api`
|
|
|
|
**รายละเอียด:**
|
|
|
|
`src/proxy.ts` ป้องกัน dashboard ทั้งหมด และ API บาง prefix เท่านั้น เช่น `/api/products`, `/api/training-records`, `/api/reports`, `/api/announcements` แต่ API กลุ่มใหม่บางส่วน เช่น `permission-templates`, `permission-audit-logs`, `user-permissions`, `permissions/catalog` ไม่ได้อยู่ใน `protectedApiPrefixes` และพึ่ง handler-level auth เป็นหลัก
|
|
|
|
ประเด็นนี้ไม่ใช่ auth bypass โดยตรง เพราะ handler หลายตัวมี `requireSuperAdminOrganizationAccess()` อยู่แล้ว แต่เป็นความไม่สม่ำเสมอของ perimeter architecture
|
|
|
|
**ผลกระทบ:**
|
|
|
|
- อ่าน boundary policy ได้ยาก
|
|
- เพิ่มความเสี่ยงที่ API ใหม่ในอนาคตจะถูกลืมเพิ่มเข้า proxy allowlist หรือทำ auth ซ้ำซ้อนแบบไม่เป็นระบบ
|
|
|
|
**หลักฐานจากโค้ด:**
|
|
|
|
- `src/proxy.ts` ระบุ `protectedApiPrefixes` แบบ hard-coded
|
|
- API permission management อยู่ใต้ `src/app/api/permission-templates`, `src/app/api/user-permissions`, `src/app/api/permission-audit-logs`, `src/app/api/permissions`
|
|
|
|
**สถานการณ์ตัวอย่าง:**
|
|
|
|
ทีมเพิ่ม API ใหม่ตาม feature module แล้วคิดว่า proxy คุ้มครองทุก API อยู่แล้ว ทั้งที่จริง route บางกลุ่มต้องพึ่ง handler guard เพียงอย่างเดียว
|
|
|
|
**คำแนะนำ:**
|
|
|
|
กำหนด perimeter strategy ให้เป็นแบบเดียว เช่น ปกป้อง API app-owned ทั้งหมดด้วย proxy matcher หรือประกาศชัดเจนว่า proxy มีไว้กัน unauthenticated only และ authorization ทั้งหมดอยู่ที่ handler
|
|
|
|
**ต้องแก้ก่อน Production หรือไม่:** ไม่ใช่
|
|
|
|
## Commands Executed and Results
|
|
|
|
1. `Get-Content -Raw docs/AI_DEVELOPMENT_GUIDE.md`
|
|
Result: ใช้เป็นเกณฑ์อ้างอิง pattern หลักของโปรเจ็กต์
|
|
2. `Get-Content -Raw docs/PROJECT_ARCHITECTURE.md`
|
|
Result: ใช้ยืนยัน target architecture และ legacy areas
|
|
3. `Get-Content -Raw plans/tms-system-full-audit-prompts.md`
|
|
Result: ใช้กำหนดรูปแบบรายงานและข้อห้ามระหว่าง audit
|
|
4. `Get-ChildItem src/features | Select-Object -ExpandProperty Name`
|
|
Result: ใช้ inventory feature modules
|
|
5. `rg -n "require(Employee|HRD|IT|Organization|SuperAdmin).*" src/app/dashboard src/app/api`
|
|
Result: ใช้สำรวจ route/page guard และ API guard ที่ใช้อยู่จริง
|
|
6. `Get-ChildItem -Recurse -Depth 2 src/app/dashboard | Select-Object FullName`
|
|
Result: ใช้ตรวจ route surface ทั้งหมดใน dashboard
|
|
7. `Get-Content -Raw src/auth.ts`
|
|
Result: พบ session enrichment และ employee-link sync
|
|
8. `Get-Content -Raw src/lib/auth/session.ts`
|
|
Result: ใช้ยืนยัน org access และ permission enforcement helper
|
|
9. `Get-Content -Raw src/lib/auth/page-guards.ts`
|
|
Result: ใช้เทียบกับ guard placement ในแต่ละ dashboard page
|
|
10. `Get-Content -Raw src/proxy.ts`
|
|
Result: ใช้ตรวจ perimeter protection ของ dashboard และ API
|
|
11. `rg -n "/dashboard/training-matrix/new" src`
|
|
Result: พบ CTA route reference เพียงจุดเดียวใน `training-matrix/page.tsx`
|
|
12. `Test-Path src/app/dashboard/training-matrix/new/page.tsx`
|
|
Result: `False`
|
|
|
|
## Files Reviewed
|
|
|
|
- `docs/AI_DEVELOPMENT_GUIDE.md`
|
|
- `docs/PROJECT_ARCHITECTURE.md`
|
|
- `plans/tms-system-full-audit-prompts.md`
|
|
- `src/auth.ts`
|
|
- `src/proxy.ts`
|
|
- `src/lib/auth/session.ts`
|
|
- `src/lib/auth/page-guards.ts`
|
|
- `src/lib/auth/roles.ts`
|
|
- `src/db/schema.ts`
|
|
- `src/app/dashboard/layout.tsx`
|
|
- `src/app/dashboard/page.tsx`
|
|
- `src/app/dashboard/overview/page.tsx`
|
|
- `src/app/dashboard/organizers/page.tsx`
|
|
- `src/app/dashboard/training-matrix/page.tsx`
|
|
- `src/app/dashboard/training-policy/page.tsx`
|
|
- `src/app/dashboard/users/page.tsx`
|
|
- `src/app/dashboard/billing/page.tsx`
|
|
- `src/app/dashboard/chat/page.tsx`
|
|
- `src/app/dashboard/react-query/page.tsx`
|
|
- `src/app/dashboard/product/page.tsx`
|
|
- `src/app/api/training-matrix/route.ts`
|
|
- `src/app/api/reports/export/route.ts`
|
|
- `src/app/api/auth/register/route.ts`
|
|
- `src/features/reports/server/report-data.ts`
|
|
- `src/features/products/components/product-listing.tsx`
|
|
- `src/features/organizers/components/organizer-listing.tsx`
|
|
- `src/features/chat/components/chat-view-page.tsx`
|
|
- `src/features/react-query-demo/components/pokemon-info.tsx`
|
|
- `src/features/employees/server/employee-identity-linking.ts`
|
|
|
|
## Areas Not Verified
|
|
|
|
- ไม่ได้รันแอปจริงใน browser เพื่อยืนยันพฤติกรรม redirect/404/runtime navigation
|
|
- ไม่ได้เชื่อมต่อฐานข้อมูลจริงเพื่อตรวจสอบ data migration state ระหว่าง `users` และ `employees`
|
|
- ไม่ได้ตรวจ coverage ของทุก route handler แบบครบทั้งระบบในเฟสนี้
|
|
- ยังไม่ได้ประเมินคุณภาพของ UI, performance, security hardening, หรือ production ops detail เพราะอยู่นอกขอบเขตของ Phase 1
|
|
|
|
## Recommended Next Step
|
|
|
|
Phase ถัดไปควรเป็น `Phase 2 - Functional Review` โดยเริ่มจาก feature inventory ของโมดูลธุรกิจหลักและแยกขอบเขต production module ออกจาก demo/template module ให้ชัด เพื่อให้ findings เชิง functional และ permission ใน phase ถัดไปไม่ปะปนกัน
|