Files
alla-tms/docs/review/employee-training-history-audit.md
2026-07-16 09:53:14 +07:00

319 lines
18 KiB
Markdown

# Employee Training History Audit Report
## Summary
Overall Status
- Good
ภาพรวม:
- หน้า `Training Records` เดินตามโครงสร้างหลักของโปรเจ็กต์ค่อนข้างดี โดยใช้ `PageContainer`, shared `DataTable`, React Query, local route handlers, และ server-side organization scoping ตามแพตเทิร์นของ `training-records`
- ฝั่ง server มีการบังคับสิทธิ์หลักถูกต้อง ทั้ง list, detail, create, update, delete และ review โดย employee เห็นได้เฉพาะข้อมูลของตนเอง ส่วน HRD/admin เห็นได้ระดับ organization
- จุดที่ควรปรับหลักอยู่ที่ความสอดคล้องระหว่าง UI กับ permission จริง, ความชัดเจนของ detail/edit flow, และการเปิดใช้ capability ฝั่ง UI ให้ครบกับ business rule เรื่อง historical filtering
---
## Architecture Review
สิ่งที่พบ:
- route หลักอยู่ที่ `src/app/dashboard/training-records/page.tsx`
- list page ใช้ server prefetch ผ่าน `src/features/training-records/components/training-record-listing.tsx`
- table ใช้ shared `DataTable`, `DataTableToolbar`, `useDataTable`
- data source ใช้ `trainingRecordsQueryOptions()` -> `api/service.ts` -> `/api/training-records`
- persistence และ access control อยู่ที่ `src/app/api/training-records/route.ts`, `src/app/api/training-records/[id]/route.ts`, และ `src/features/training-records/server/training-record-data.ts`
- detail route reuse form page ผ่าน `src/features/training-records/components/training-record-view-page.tsx`
ข้อสรุป:
- โครงสร้างโดยรวมสอดคล้องกับเอกสาร `AI_DEVELOPMENT_GUIDE` และ canonical feature ของโปรเจ็กต์
- อย่างไรก็ดี semantic ของหน้า detail/edit ยังไม่ชัด เพราะ route เดียวกันถูกใช้ทั้ง "ดูรายละเอียด" และ "แก้ไข"
---
## Functional Review
| Feature | Status | Notes |
| ------- | ------ | ----- |
| View own training history | Pass | page ใช้ `requireEmployeeDashboardAccess()` และ list query ถูก scope ฝั่ง server |
| Search records | Pass | รองรับ text search ผ่าน table toolbar |
| Filter by status | Pass | UI และ API ทำงานสอดคล้องกัน |
| Filter by training type | Pass | UI และ API ทำงานสอดคล้องกัน |
| Sort records | Pass | default sort ตาม `training_date desc` และรองรับ server-side sort |
| Pagination | Pass | ใช้ shared DataTable pagination |
| Create training record | Pass | ใช้ TanStack Form, validation, mutation, และ certificate upload ต่อเนื่อง |
| Edit pending record | Partial | server guard ถูกต้อง แต่ client affordance กว้างกว่า permission บางกรณี |
| Delete pending record | Partial | server guard ถูกต้อง แต่ client affordance กว้างกว่า permission บางกรณี |
| View detail vs update flow | Partial | action `Detail` และ `Update` พาไป route เดียวกัน |
| Historical filtering by year | Partial | backend รองรับ `year` filter แต่หน้า employee training history ยังไม่ expose |
| Loading state | Pass | มี `loading.tsx` ทั้ง list และ detail |
| Error state | Partial | list/detail route ไม่มี dedicated error boundary และ review error page แสดง raw error |
| Empty state | Partial | มี empty state แต่ยังไม่อธิบายกรณี employee profile ไม่ถูกผูกกับ user |
---
## Business Rule Review
สิ่งที่สอดคล้อง:
- employee เห็นเฉพาะ record ของตนเองผ่าน `buildTrainingRecordAccessClause()` และ `getEmployeeIdsForUser()`
- HRD/admin เห็นข้อมูลได้ทั้ง organization
- employee แก้ไขหรือลบได้เฉพาะ record ที่ยัง `pending` ตาม server rule `canEmployeeManagePendingRecord()`
- duration ถูก normalize และ serialize สม่ำเสมอผ่าน minute-based helpers
จุดที่ควรระวัง:
- business rule เรื่องการแยกประวัติเป็นรายปีมี support ฝั่ง API (`year`) แต่หน้า employee training history ยังไม่เปิดให้ใช้จาก UI
- หาก record เป็น `pending` แต่ไม่มี `user_id` ผูกกับผู้ใช้งานปัจจุบัน UI ยังดูเหมือนแก้ไขได้ ทั้งที่ API จะปฏิเสธ
---
## Role & Permission Review
Employee
- View own list: Pass
- View own detail: Pass
- Create own record: Pass
- Edit own pending record: Partial
- Delete own pending record: Partial
- Review records: Guarded
- View other employees: Guarded server-side
HRD / Admin
- View organization-scoped training records: Pass
- Review records: Pass
- Create/update/delete records: Pass ตาม role model ปัจจุบัน
ข้อสังเกต:
- ฝั่ง server permission ค่อนข้างแข็งแรง
- ฝั่ง UI มี permission drift บางจุด เพราะ action menu และ form ใช้เงื่อนไข `approval_status === 'pending'` เป็นหลัก มากกว่าการยืนยัน ownership แบบเดียวกับ API
---
## UI Review
จุดที่ดี:
- ใช้ shared DataTable stack สม่ำเสมอ
- list page responsive ในระดับดี และจำกัด overflow ไว้ใน table shell
- loading skeleton ของ list/detail ชัดเจนและสอดคล้องกับหน้า dashboard อื่น
จุดที่ควรปรับ:
- action `Detail` กับ `Update` ใช้ปลายทางเดียวกัน ทำให้ label กับพฤติกรรมไม่ตรงกัน
- empty state เป็น generic "ยังไม่มีประวัติ" โดยไม่แยกกรณี "ยังไม่ผูก employee profile"
- detail route แสดงเป็นฟอร์มเต็มทันที แม้ผู้ใช้เพียงต้องการอ่านข้อมูล
---
## UX Review
จุดที่ดี:
- flow สร้าง record ใหม่ตรงไปตรงมา
- หลัง create/update มี toast และ redirect กลับ list
- search/filter/sort อยู่ใน pattern เดียวกับ feature อื่น ทำให้คาดเดาได้
จุดที่ควรปรับ:
- ผู้ใช้บางรายอาจกด `Update` หรือ `Delete` ได้จาก menu แต่ไปชน 403 ตอนเรียก API จริง
- ไม่มีความต่างชัดเจนระหว่าง "ดูรายละเอียด" กับ "แก้ไข"
- หน้า list ยังขาด filter รายปี ทั้งที่บริบทธุรกิจของ training history มักอิงปีปฏิทิน
---
## Accessibility Review
จุดที่ดี:
- action menu trigger มี `sr-only` label
- ฟิลด์หลักในฟอร์มมี label และ `aria-invalid`
- ปุ่มหลักและ loading skeleton อยู่ในโครงสร้างที่อ่านได้
จุดที่ควรระวัง:
- ความต่างระหว่าง badge status ยังพึ่งข้อความมากกว่า visual emphasis
- การใช้ route เดียวกันสำหรับ detail/edit อาจทำให้ mental model ของผู้ใช้และ assistive flow สับสน
---
## Code Quality Review
จุดที่ดี:
- แยก `types.ts`, `service.ts`, `queries.ts`, `mutations.ts`, `server` helpers ชัดเจน
- route handlers บางและส่งงาน query logic ไปที่ feature server helper
- ใช้ shared form/table stack ตาม guideline
จุดที่ควรสังเกต:
- permission logic ฝั่ง client ซ้ำแต่ไม่เทียบเท่ากับ server rule
- naming ของ detail/update flow ยังไม่สะท้อน behavior จริง
---
## API Review
จุดที่ดี:
- contracts ของ list/detail ชัด
- route handlers ใช้ `requireOrganizationAccess()` และคืนค่า error ที่สม่ำเสมอพอสมควร
- list endpoint รองรับ filter มากกว่าที่ UI ปัจจุบันใช้
จุดที่ควรปรับ:
- client surface ยังใช้ filter contract ไม่ครบ โดยเฉพาะ `year`
- review error UI แสดง `error.message` ตรง ๆ ต่อผู้ใช้
---
## Performance Review
จุดที่ดี:
- list page ใช้ server prefetch + hydration
- table ใช้ server-side pagination/sorting/filtering
- detail page prefetch เฉพาะ query ที่เกี่ยวข้อง
จุดที่ควรระวัง:
- list query สร้าง certificate preview map ให้ทุก row ซึ่งสมเหตุผลกับคอลัมน์ปัจจุบัน แต่ถ้าอนาคตมี lightweight employee view แยกจาก management view อาจแยก payload ได้อีก
---
## Security Review
จุดที่ดี:
- page ใช้ route guard ก่อน render
- API ใช้ organization access check และ server-side record scoping
- update/delete บังคับ pending-only และ ownership เฉพาะ employee
ความเสี่ยง:
- review error page แสดง raw runtime error ต่อผู้ใช้
---
## Consistency Review
สอดคล้อง:
- ใช้ `PageContainer`
- ใช้ shared DataTable stack
- ใช้ TanStack Form + project wrappers
- ใช้ route handlers + Drizzle ตาม pattern กลาง
ไม่สอดคล้อง:
- detail page ของ training history ยังไม่แยก read-only view ออกจาก edit semantics
- UI permission checks ฝั่ง client ยังไม่ align กับ server helper เดียวกัน
---
## Edge Case Review
- New employee with no history: รองรับได้ แต่ empty state ยัง generic
- Employee without linked profile: create flow block ได้ แต่ list/empty state ไม่อธิบายสาเหตุ
- Pending record without linked `user_id`: มีความเสี่ยงด้าน UX เพราะ UI อาจเปิด action แต่ API ปฏิเสธ
- Approved record: server กัน edit/delete ถูกต้อง
- No certificate: รองรับได้
- API failure: review route มี error UI แต่เปิด raw error; list/detail ไม่มี dedicated boundary
- Slow network: loading skeleton มีให้ทั้ง list และ detail
---
## Problems Found
### High
- Description: Client-side action และ form affordance อนุญาตให้ employee เห็น `Update/Delete` จากเงื่อนไข `approval_status === 'pending'` แม้ API จะอนุญาตเฉพาะ record ที่ `recordUserId === actorUserId`
- File: `src/features/training-records/components/training-record-tables/cell-action.tsx`, `src/features/training-records/components/training-record-form.tsx`, `src/features/training-records/server/training-record-data.ts`, `src/app/api/training-records/[id]/route.ts`
- Component: `CellAction`, `TrainingRecordForm`, `canEmployeeManagePendingRecord`, `PATCH/DELETE /api/training-records/[id]`
- Severity: High
- Impact: ผู้ใช้สามารถเข้าสู่ flow แก้ไข/ลบได้ แต่ลงท้ายด้วย 403 เมื่อ record เป็น pending ที่ยังไม่ผูก `user_id` อย่างครบถ้วน ทำให้ permission model ใน UI กับ backend ไม่ตรงกัน
- Recommendation: ให้ client reuse permission contract เดียวกับ server หรือส่ง capability flag จาก API เพื่อไม่ให้ action menu/submit state over-promise
### Medium
- Description: หน้า employee training history ยังไม่ expose `year` filter ทั้งที่ API, types, และ server helper รองรับแล้ว
- File: `src/app/dashboard/training-records/page.tsx`, `src/features/training-records/components/training-record-listing.tsx`, `src/features/training-records/components/training-record-tables/index.tsx`, `src/features/training-records/api/types.ts`, `src/app/api/training-records/route.ts`
- Component: page route, listing page, `TrainingRecordsTable`, route handler contract
- Severity: Medium
- Impact: ผู้ใช้ยังแยกดูประวัติรายปีจากหน้า training history ไม่ได้ ทั้งที่เป็น use case ธุรกิจหลักของประวัติการอบรม
- Recommendation: เปิด filter รายปีใน UI และเชื่อมกับ query state เดิมของหน้า
- Description: `Detail` และ `Update` action พาไป route เดียวกัน และ route นั้น render ฟอร์มแก้ไข/อ่านแบบเดียวกัน
- File: `src/features/training-records/components/training-record-tables/cell-action.tsx`, `src/features/training-records/components/training-record-view-page.tsx`
- Component: `CellAction`, `TrainingRecordViewPage`
- Severity: Medium
- Impact: semantics ของหน้ารายละเอียดไม่ชัด ผู้ใช้ไม่สามารถแยกได้ว่าเข้ามาเพื่ออ่านหรือเพื่อแก้ไข และ flow นี้ทำให้ UX ของ training history ดูคลุมเครือ
- Recommendation: แยก read-only detail state ให้ชัด หรือปรับ label/action ให้ตรงพฤติกรรมจริง
- Description: list/detail route ของ training history ไม่มี dedicated error boundary ขณะที่ review error page แสดง raw `error.message`
- File: `src/app/dashboard/training-records`, `src/app/dashboard/training-records/[trainingRecordId]/review/error.tsx`
- Component: training history route, `TrainingRecordReviewError`
- Severity: Medium
- Impact: เมื่อ query/render พัง ผู้ใช้ได้ UX ที่ไม่สม่ำเสมอ และบางกรณีอาจเห็นข้อความเทคนิคจาก runtime
- Recommendation: เพิ่ม route-level error UI ที่ปลอดภัยและใช้ข้อความสำหรับผู้ใช้ปลายทาง
- Description: empty state ของหน้า list ยังไม่แยกกรณี "ไม่มีประวัติ" ออกจาก "บัญชียังไม่ถูกผูกกับ employee profile"
- File: `src/features/training-records/components/training-record-tables/index.tsx`, `src/features/training-records/components/training-record-form.tsx`
- Component: `TrainingRecordsTable`, `TrainingRecordForm`
- Severity: Medium
- Impact: ผู้ใช้และทีม support แยกสาเหตุของข้อมูลว่างได้ยาก
- Recommendation: เพิ่ม explanatory state หรือ guidance สำหรับบัญชีที่ยังไม่ผูก employee profile
### Low
- Description: status badge ใน table ใช้ `outline` เหมือนกันทุกสถานะ
- File: `src/features/training-records/components/training-record-tables/columns.tsx`
- Component: `columns`
- Severity: Low
- Impact: การสแกนสถานะด้วยสายตาทำได้ช้ากว่าที่ควร
- Recommendation: เพิ่ม visual distinction ระหว่าง `pending`, `approved`, `rejected`, `needs_revision`
---
## Missing Features
- filter รายปีสำหรับ employee training history
- dedicated error UI สำหรับ list/detail route
- explanatory empty state สำหรับบัญชีที่ยังไม่ผูก employee profile
- clear separation ระหว่าง read-only detail กับ edit flow
---
## Improvement Opportunities
- ส่ง permission/capability flags จาก API เพื่อให้ UI action menu ตรงกับ server policy เสมอ
- แยก employee-facing training history semantics ออกจาก management/edit semantics ให้ชัดขึ้น
- พิจารณาเพิ่ม lightweight read-only summary row/detail state สำหรับ employee
- ปรับ labels และ CTA ให้สะท้อน flow จริง เช่น detail, edit, review
---
## Final Assessment
Architecture: 8.2/10
Functionality: 7.4/10
UI: 7.2/10
UX: 6.9/10
Accessibility: 7.0/10
Performance: 7.8/10
Security: 8.0/10
Maintainability: 7.5/10
Overall Score: 7.5/10