allagroup/alla-tms

Fork 0

Files

ALLAGROUP\mtpnuniyo 29cec708a3 commit

2026-07-16 09:53:14 +07:00

11 KiB

Raw Permalink Blame History

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

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:

High

None open after the file-access fix.

Medium

Notification listing is not paginated efficiently at the database level
Evidence:
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
Audit coverage for certificate changes was incomplete
Status: Fixed in this review

Fixed in:
- src/app/api/training-records/[id]/certificates/route.ts
- src/app/api/training-records/[id]/certificates/[certificateId]/route.ts

Low

RBAC documentation is outdated and still references Clerk and client-side authorization concepts
Evidence:
Risk:
- Future maintenance confusion
- Incorrect onboarding assumptions for access control changes
Announcements route segment does not yet have route-level loading.tsx / error.tsx coverage
Evidence:
- src/app/dashboard/announcements/page.tsx
- 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:

Notification pagination is currently in-memory after loading all rows
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

Protected announcement attachment access
Protected certificate download access
Added audit logging for certificate upload/delete
- src/app/api/training-records/[id]/certificates/route.ts
- src/app/api/training-records/[id]/certificates/[certificateId]/route.ts

Database Changes

No database schema changes were required.

Recommended Fixes Before Sprint 7

Move notification type filtering and pagination fully into SQL
Update docs/nav-rbac.md to reflect Auth.js + current server-side authorization model
Add route-level loading.tsx / error.tsx consistency for announcements
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.

11 KiB Raw Permalink Blame History