256 lines
11 KiB
Markdown
256 lines
11 KiB
Markdown
# Pre-UAT Readiness Review
|
|
|
|
Project: Training Management System (TMS)
|
|
Review date: 2026-06-18
|
|
Scope: Sprint 1-6 modules before Sprint 7
|
|
|
|
## Executive Summary
|
|
|
|
The system is close to UAT-ready. Core authentication, organization scoping, training records, HRD review, dashboard, import, announcements, notifications, and responsive work are in place and the main protected routes use server-side session checks.
|
|
|
|
The most important pre-UAT risk found in this review was file exposure through direct `public/uploads/...` URLs for certificates and announcement attachments. That issue has been fixed in code during this review by moving user-facing file access to protected API download routes with authorization checks.
|
|
|
|
After that fix, there are no remaining critical blockers for UAT. The main open items are performance hardening in notifications, documentation drift in RBAC documentation, and a few UX/state consistency gaps that should be cleaned up before production hardening.
|
|
|
|
## UAT Readiness Score
|
|
|
|
- UAT Readiness: **87/100**
|
|
- Go-Live Readiness: **81/100**
|
|
|
|
Interpretation:
|
|
|
|
- Good candidate for controlled UAT
|
|
- Not yet ideal for unrestricted production rollout without another hardening pass
|
|
|
|
## Issues By Severity
|
|
|
|
### Critical
|
|
|
|
1. Direct access to uploaded certificates and announcement attachments bypassed business authorization
|
|
Status: **Fixed in this review**
|
|
|
|
Previous behavior:
|
|
- Certificate and announcement files were stored under `public/uploads/...`
|
|
- Serialized API responses exposed direct public URLs
|
|
- Anyone with the path could access the file without record-level authorization
|
|
|
|
Fixed in:
|
|
- [src/app/api/training-records/[id]/certificates/[certificateId]/download/route.ts](/d:/WY-2569/HRD/training-system-minimal-refactor/src/app/api/training-records/[id]/certificates/[certificateId]/download/route.ts)
|
|
- [src/app/api/announcements/[id]/attachment/route.ts](/d:/WY-2569/HRD/training-system-minimal-refactor/src/app/api/announcements/[id]/attachment/route.ts)
|
|
- [src/features/training-records/server/training-record-data.ts](/d:/WY-2569/HRD/training-system-minimal-refactor/src/features/training-records/server/training-record-data.ts)
|
|
- [src/features/announcements/server/announcement-data.ts](/d:/WY-2569/HRD/training-system-minimal-refactor/src/features/announcements/server/announcement-data.ts)
|
|
- [src/app/api/training-records/[id]/certificates/route.ts](/d:/WY-2569/HRD/training-system-minimal-refactor/src/app/api/training-records/[id]/certificates/route.ts)
|
|
|
|
### High
|
|
|
|
None open after the file-access fix.
|
|
|
|
### Medium
|
|
|
|
1. Notification listing is not paginated efficiently at the database level
|
|
Evidence:
|
|
- [src/features/notifications/server/notification-data.ts:77](/d:/WY-2569/HRD/training-system-minimal-refactor/src/features/notifications/server/notification-data.ts:77)
|
|
- [src/features/notifications/server/notification-data.ts:83](/d:/WY-2569/HRD/training-system-minimal-refactor/src/features/notifications/server/notification-data.ts:83)
|
|
- [src/features/notifications/server/notification-data.ts:85](/d:/WY-2569/HRD/training-system-minimal-refactor/src/features/notifications/server/notification-data.ts:85)
|
|
|
|
Current behavior:
|
|
- The service loads all matching rows
|
|
- Filters notification type in memory
|
|
- Then slices the array for pagination
|
|
|
|
Risk:
|
|
- Will degrade as notification volume grows
|
|
- Can affect dashboard header responsiveness and notification panel latency during UAT with larger datasets
|
|
|
|
2. Audit coverage for certificate changes was incomplete
|
|
Status: **Fixed in this review**
|
|
|
|
Fixed in:
|
|
- [src/app/api/training-records/[id]/certificates/route.ts](/d:/WY-2569/HRD/training-system-minimal-refactor/src/app/api/training-records/[id]/certificates/route.ts)
|
|
- [src/app/api/training-records/[id]/certificates/[certificateId]/route.ts](/d:/WY-2569/HRD/training-system-minimal-refactor/src/app/api/training-records/[id]/certificates/[certificateId]/route.ts)
|
|
|
|
### Low
|
|
|
|
1. RBAC documentation is outdated and still references Clerk and client-side authorization concepts
|
|
Evidence:
|
|
- [docs/nav-rbac.md:5](/d:/WY-2569/HRD/training-system-minimal-refactor/docs/nav-rbac.md:5)
|
|
- [docs/nav-rbac.md:7](/d:/WY-2569/HRD/training-system-minimal-refactor/docs/nav-rbac.md:7)
|
|
- [docs/nav-rbac.md:19](/d:/WY-2569/HRD/training-system-minimal-refactor/docs/nav-rbac.md:19)
|
|
- [docs/nav-rbac.md:79](/d:/WY-2569/HRD/training-system-minimal-refactor/docs/nav-rbac.md:79)
|
|
|
|
Risk:
|
|
- Future maintenance confusion
|
|
- Incorrect onboarding assumptions for access control changes
|
|
|
|
2. Announcements route segment does not yet have route-level `loading.tsx` / `error.tsx` coverage
|
|
Evidence:
|
|
- [src/app/dashboard/announcements/page.tsx](/d:/WY-2569/HRD/training-system-minimal-refactor/src/app/dashboard/announcements/page.tsx)
|
|
- [src/app/dashboard/announcements/[id]/page.tsx](/d:/WY-2569/HRD/training-system-minimal-refactor/src/app/dashboard/announcements/[id]/page.tsx)
|
|
|
|
Note:
|
|
- Component-level loading/error handling already exists, so this is not a blocker
|
|
- This is mainly consistency debt versus other dashboard modules
|
|
|
|
## Security Review
|
|
|
|
### Authentication
|
|
|
|
- `src/proxy.ts` protects `/dashboard` and protected API namespaces
|
|
- Route handlers consistently use `requireOrganizationAccess()`, `requireHRD()`, or page guards
|
|
- Auth.js session remains the source of truth
|
|
|
|
Assessment: **Pass**
|
|
|
|
### Authorization
|
|
|
|
- Training records are self-scoped for employee users and organization-scoped for HRD/admin
|
|
- Review actions enforce elevated role checks
|
|
- Announcement visibility is correctly restricted for employee users to active published announcements
|
|
- New protected file download routes now enforce authorization before serving attachments
|
|
|
|
Assessment: **Pass after fix**
|
|
|
|
### File Upload Security
|
|
|
|
- File type and size validation exist for certificates and announcements
|
|
- Storage path traversal protection exists via root-path normalization checks
|
|
- Previous public direct-file exposure risk is now mitigated by protected download endpoints
|
|
|
|
Assessment: **Pass after fix**
|
|
|
|
## Authorization Review
|
|
|
|
Validated behaviors:
|
|
|
|
- Employee can only access own training records through server-side filtering
|
|
- HRD/admin can review organization records
|
|
- Employee edit/delete remains limited to pending records
|
|
- Certificate upload/delete remains limited to pending records
|
|
- Employee cannot open HRD-only announcement creation flow
|
|
|
|
Assessment: **Good**
|
|
|
|
## Validation Review
|
|
|
|
- Zod validation is present in training review and announcement mutation flows
|
|
- Training record mutation normalization is in place
|
|
- Notification query validation is present
|
|
- Import flow validates template headers, required columns, duplicate rows, email format, and status values
|
|
|
|
Assessment: **Good**
|
|
|
|
## Audit And Notification Review
|
|
|
|
Audit:
|
|
|
|
- Core CRUD and review actions are logged
|
|
- Certificate upload/delete logging was added in this review
|
|
- Notification read / mark-all-read actions are logged
|
|
|
|
Notifications:
|
|
|
|
- Review approval/rejection notifications exist
|
|
- Announcement publish notifications exist
|
|
- Import completion notifications exist
|
|
|
|
Assessment: **Good, with performance follow-up needed in notification retrieval**
|
|
|
|
## Dashboard Accuracy Review
|
|
|
|
- Employee dashboard calculations use approved records for approved-hour KPIs
|
|
- HRD summary and chart queries scope by organization and optional filters
|
|
- Pending counts are filtered by year and selected scope
|
|
|
|
Assessment: **Pass based on current query logic review**
|
|
|
|
Residual note:
|
|
|
|
- This review was code-based. Final confidence should be reinforced with UAT dataset walkthroughs covering mixed approved/pending/rejected records across multiple departments.
|
|
|
|
## Responsive Review
|
|
|
|
Based on prior responsive and mobile UX passes plus current spot checks:
|
|
|
|
- Dashboard and training-record surfaces have mobile/tablet-aware layout work
|
|
- Tables and action groups use responsive stacking in key modules
|
|
- Certificate management actions are responsive
|
|
|
|
Assessment: **Good for UAT**
|
|
|
|
Residual note:
|
|
|
|
- Announcement pages should receive the same route-level loading/error consistency as other modules
|
|
|
|
## Performance Review
|
|
|
|
Strengths:
|
|
|
|
- Server-side queries are mostly scoped properly
|
|
- Overview data uses bounded yearly aggregations
|
|
- Query prefetch + hydration patterns are used consistently
|
|
|
|
Open concerns:
|
|
|
|
1. Notification pagination is currently in-memory after loading all rows
|
|
2. Large file downloads are still served from local app storage and app process memory
|
|
|
|
Assessment: **Acceptable for UAT, not yet optimized for scale**
|
|
|
|
## Browser / Runtime Confidence
|
|
|
|
What was verified in this review:
|
|
|
|
- TypeScript compile passed: `npx tsc --noEmit`
|
|
- Lint passed with existing non-blocking warnings: `npm run lint`
|
|
|
|
Not fully re-verified in this review:
|
|
|
|
- Cross-browser manual testing
|
|
- End-to-end download behavior in browser
|
|
- Full mobile interaction regression pass
|
|
|
|
Assessment: **Moderate confidence; recommend targeted smoke test before UAT handoff**
|
|
|
|
## Code Changes Applied In This Review
|
|
|
|
1. Protected announcement attachment access
|
|
- [src/app/api/announcements/[id]/attachment/route.ts](/d:/WY-2569/HRD/training-system-minimal-refactor/src/app/api/announcements/[id]/attachment/route.ts)
|
|
- [src/lib/announcement-storage.ts](/d:/WY-2569/HRD/training-system-minimal-refactor/src/lib/announcement-storage.ts)
|
|
- [src/features/announcements/server/announcement-data.ts](/d:/WY-2569/HRD/training-system-minimal-refactor/src/features/announcements/server/announcement-data.ts)
|
|
|
|
2. Protected certificate download access
|
|
- [src/app/api/training-records/[id]/certificates/[certificateId]/download/route.ts](/d:/WY-2569/HRD/training-system-minimal-refactor/src/app/api/training-records/[id]/certificates/[certificateId]/download/route.ts)
|
|
- [src/lib/certificate-storage.ts](/d:/WY-2569/HRD/training-system-minimal-refactor/src/lib/certificate-storage.ts)
|
|
- [src/features/training-records/server/training-record-data.ts](/d:/WY-2569/HRD/training-system-minimal-refactor/src/features/training-records/server/training-record-data.ts)
|
|
- [src/app/api/training-records/[id]/certificates/route.ts](/d:/WY-2569/HRD/training-system-minimal-refactor/src/app/api/training-records/[id]/certificates/route.ts)
|
|
|
|
3. Added audit logging for certificate upload/delete
|
|
- [src/app/api/training-records/[id]/certificates/route.ts](/d:/WY-2569/HRD/training-system-minimal-refactor/src/app/api/training-records/[id]/certificates/route.ts)
|
|
- [src/app/api/training-records/[id]/certificates/[certificateId]/route.ts](/d:/WY-2569/HRD/training-system-minimal-refactor/src/app/api/training-records/[id]/certificates/[certificateId]/route.ts)
|
|
|
|
## Database Changes
|
|
|
|
No database schema changes were required.
|
|
|
|
## Recommended Fixes Before Sprint 7
|
|
|
|
1. Move notification type filtering and pagination fully into SQL
|
|
2. Update `docs/nav-rbac.md` to reflect Auth.js + current server-side authorization model
|
|
3. Add route-level `loading.tsx` / `error.tsx` consistency for announcements
|
|
4. Run a focused browser smoke test for:
|
|
- employee certificate open/download
|
|
- HRD announcement attachment open/download
|
|
- approval/rejection notifications
|
|
- mixed dashboard KPI scenarios
|
|
|
|
## Final Readiness Position
|
|
|
|
Recommendation: **Proceed to UAT**
|
|
|
|
Conditions:
|
|
|
|
- Use the current build with the file-access fix included
|
|
- Complete a short smoke test pass on attachments, notifications, and dashboard totals before handing to business users
|
|
|
|
This is a reasonable UAT baseline. It still needs one more hardening pass before production-grade confidence.
|