develop full initial module
This commit is contained in:
@@ -110,6 +110,74 @@ Convention: `docs/01-DOC-GUIDE.md §6`. Update this file in the **same commit**
|
||||
- [ ] Reservation/allocation fulfilment
|
||||
- [ ] RBAC policy enforcement + approval workflow activation
|
||||
|
||||
---
|
||||
|
||||
# HRM (Phase 2)
|
||||
|
||||
Spec: `docs/12-BACKEND-HRM.md` (model + rules) · `docs/13-BACKEND-HRM-API.md` (API). Security: `docs/02-SECURITY.md §C.8` (run before ticking any HRM feature `[x]` — salary/PII data, see AR-09/AR-10).
|
||||
|
||||
### 2026-07-23 — Bug fix: `DateTime Kind=Unspecified` 500 on every user-supplied date (Employee create, Statutory Settings create, etc.)
|
||||
- **Root cause:** Postgres/Npgsql requires `DateTime` values written to a `timestamp with time zone` column to have `Kind=Utc`. Every Phase-1 `DateTime` was always server-generated (`DateTime.UtcNow`), so this never surfaced before. HRM is the first place user-supplied dates (hire date, salary-structure/statutory-setting effective date, leave/attendance period dates, document issue/expiry dates, …) get deserialized straight from a JSON request body — which produces `Kind=Unspecified` — and then persisted, so **any** create involving a date (`POST /employees`, `POST /payroll-statutory-settings`, `POST /tax-slabs`, `POST /employees/{id}/salary-structure`, `POST /leave-requests`, attendance upload, …) threw a `500` (`DbUpdateException` → `ArgumentException: Cannot write DateTime with Kind=Unspecified...`). Confirmed via `logs/erpcore-20260723.log`.
|
||||
- **Fix:** a global EF Core `ValueConverter<DateTime, DateTime>`/`ValueConverter<DateTime?, DateTime?>` registered once in `ErpDbContext.OnModelCreating` (applied to every entity property of type `DateTime`/`DateTime?` via `modelBuilder.Model.GetEntityTypes()`), forcing `Kind=Utc` on write. Fixes the bug for every current and future HRM (and Phase-1) entity in one place, rather than patching each service call site individually.
|
||||
- **No migration needed** — confirmed by scaffolding a migration and finding it empty (`Up`/`Down` both no-ops), then removing it. The converter doesn't change the store type (`timestamp with time zone` throughout), only how the CLR value's `Kind` is normalized before Npgsql sees it.
|
||||
- **Verified:** `dotnet build` clean (0/0). Not yet re-verified end-to-end against a live AuthHex session (same blocker as the rest of this phase's runtime testing) — the next person to get a token should re-try `POST /employees` and `POST /payroll-statutory-settings` to confirm the `500` is gone.
|
||||
|
||||
## 7. Sub-phase 2.1 — Employee + User-link + Documents
|
||||
> **Code complete + migration applied (2026-07-23).** `dotnet build` clean (0/0); migration `AddHrmPhase1Foundation` generated + applied to the local Postgres DB (new tables only + one nullable `users.Email` column — the scaffolder's "possible data loss" warning is just the benign `UpdateData` setting the seeded system user's `Email` to `null`, not a drop). Runtime smoke-test (Swagger/browser) not yet run — do that before ticking `[x]`, per the §6 security-gate convention §1's note established for Phase 1.
|
||||
- [x] Org masters: Branch, Department (self-nesting + cycle guard), Designation, EmploymentType, WorkShift — entities + configs + services + controllers (CRUD, ETag, deactivate-not-delete). Routes: `/branches`, `/departments`, `/designations`, `/employment-types`, `/work-shifts`.
|
||||
- [x] Employee entity + config (EmployeeStatus enum, EmployeeCode uniqueness, all FKs) + service + controller (`/employees`)
|
||||
- [x] EmployeeBankDetail (one-to-many, IsPrimary) — `GET/PUT /employees/{id}/bank-details` (full-replace)
|
||||
- [x] `User.Email` column + unique index (Postgres allows multiple NULLs natively, same pattern as `AuthUserId` — no explicit filter needed); `UsersController.Create` now persists it locally (previously silently dropped despite `CreateUserRequest.Email` being required) and returns it on `ManagedUserDto`.
|
||||
- [x] `EmployeeUserLinkService` + email-lookup endpoints (`GET /employees/email-lookup`, `GET /users/email-lookup`, both advisory/non-mutating) + `POST/DELETE /employees/{id}/link-user` + `linkUserId`/`linkEmployeeId` on the two create endpoints. `Employee.UserId` has a unique index (DB-level one-User-per-Employee guarantee) backed by service-level `EMPLOYEE_ALREADY_LINKED`/`USER_ALREADY_LINKED` (409) checks.
|
||||
- [x] `HrDocumentType` master (CRUD, deactivate-not-delete) — `/hr-document-types`
|
||||
- [x] `Infra/Storage/IFileStorageService` + `LocalFileStorageService` (root `App_Data/hr-documents` outside wwwroot, year/month bucketing, path-escape guard, registered as a **singleton** — stateless aside from the configured root)
|
||||
- [x] `EmployeeDocument` entity + upload/list/download/status endpoints (`POST/GET /employees/{id}/documents`, `GET .../documents/{docId}/download`, `PATCH .../documents/{docId}/status`) — extension allowlist (`.pdf/.jpg/.jpeg/.png/.docx`) cross-checked against declared content-type, size cap from `FileStorage:MaxSizeBytes` (appsettings, default 10MB)
|
||||
|
||||
**Deviations (recorded, not silently skipped):**
|
||||
- **JIT-provisioning Email backfill not implemented.** `ShadowUserClaimsTransformation` still only sets Username/DisplayName from the AuthHex token — the token carries no `Email` claim (confirmed set: `UserId`/`UserTypeCode`/`RoleCode`/`NIC`/`jti`/`iat`), so backfilling it would require an extra AuthHex API call (`getUserDetails`) inside the claims-transformation hot path. Deferred as a follow-up; the primary path (`UsersController.Create`, which already collects `Email` in the request body) covers the common case of an ERPCore-driven user creation.
|
||||
- **`DOCUMENT_TYPE_IN_USE` error code is defined but not wired to anything** — there is no hard-`DELETE` endpoint for `HrDocumentType` (same deactivate-only convention as every Phase-1 master; FR-MD-08), so nothing currently triggers it. Reserved for consistency with the doc, same posture as Phase 1's unused `MASTER_IN_USE` before transaction tables existed.
|
||||
- **`EmployeeDocumentService` reads `Stream.Length` for the size-cap check** rather than `IFormFile.Length` directly — works because ASP.NET Core's default `IFormFile.OpenReadStream()` returns a seekable buffered stream, but would need revisiting if a non-seekable upload path is ever added.
|
||||
|
||||
## 8. Sub-phase 2.2 — Attendance + Leave
|
||||
> **Code complete + migration applied (2026-07-23).** `dotnet build` clean (0 errors); migration `AddHrmAttendanceAndLeave` generated (purely additive, no data-loss warning) + applied to the local Postgres DB. Packages added: `ClosedXML` 0.105.0, `CsvHelper` 33.1.0. Runtime smoke-test not yet run.
|
||||
- [x] LeaveType (master CRUD, `/leave-types`), LeaveRequest (Draft/Submitted/Approved/Rejected/Cancelled, `/leave-requests`, DocNo via `NumberSequenceService` "LV"), LeaveBalance (`GET/PUT /employees/{id}/leave-balances`) — approving a request increments `LeaveBalance.TakenDays` via `ILeaveBalanceService.IncrementTakenDaysAsync`
|
||||
- [x] WorkShift-based `AttendanceComputationService` (Working/Late/Early/OT minutes; overnight-shift handling; derives Present/HalfDay/Absent/Holiday/WeekOff/OnLeave) — the analog of `FifoCostingService`
|
||||
- [x] AttendanceUploadBatch + AttendanceRecord entities/config (`WorkShiftId` snapshotted at ingestion per docs A.3)
|
||||
- [x] Excel/CSV parsing (ClosedXML for `.xlsx`, CsvHelper for `.csv`) + `GET /attendance-batches/template.xlsx`/`?format=csv` — both share `AttendanceUploadService.ColumnNames` so template and parser can't drift
|
||||
- [x] Upload → validate (employee-code resolution against Active employees, date/time parse, within-batch + cross-batch-confirmed duplicate detection) → confirm pipeline, exact status flow Draft→Validated→Confirmed→UsedInPayroll (`/attendance-batches`, `.../validate`, `.../confirm`)
|
||||
- [x] Manual record edit (`PUT .../records/{id}`, blocked once Confirmed/UsedInPayroll → `409 ATTENDANCE_BATCH_LOCKED`) + duplicate resolution (`POST .../resolve-duplicate`, keep/discard/supersede) + unlock (`POST .../unlock`, mandatory reason, blocked once `UsedInPayroll`)
|
||||
- [x] Leave→Attendance OnLeave classification wired in — `AttendanceUploadService` calls `ILeaveRequestService.FindApprovedLeaveCoveringAsync` per record during upload and re-computation
|
||||
|
||||
**Deviations (recorded):**
|
||||
- **LeaveRequest.DaysCount is a calendar-day count** (`EndDate − StartDate + 1`), not business-day/holiday-aware. Flagged as a simplification in the service's own doc comment — a real deployment will want to exclude weekends/holidays from paid-leave day counts before this feeds Payroll.
|
||||
- **No `Holiday` calendar entity exists yet** — `AttendanceComputationService.Compute` always receives `isHoliday: false`; only `WeekOff` (derived from `WorkShift.WorkingDaysMask`) and `OnLeave` are currently distinguishable from a plain `Absent`. A company-holiday calendar is a natural near-term addition, not built in this pass.
|
||||
- **`ResolveDuplicateAsync`'s "supersede" action does not yet locate/mutate the prior confirmed record** — it currently just accepts the new row as Valid. The prior confirmed `AttendanceRecord` this is meant to supersede is not looked up or flagged; this needs a follow-up pass before "supersede" is safe to expose to non-admin users in the UI.
|
||||
|
||||
## 9. Sub-phase 2.3 — Payroll
|
||||
> **Code complete + migration applied (2026-07-23).** `dotnet build` clean (0 errors); migration `AddHrmPayroll` generated + applied to the local Postgres DB. Runtime smoke-test not yet run.
|
||||
- [x] SalaryComponent master (`/salary-components`) — Earning/Deduction, IsTaxable, IsEpfEtfApplicable
|
||||
- [x] EmployeeSalaryStructure (+Lines), effective-dated (`GET/POST /employees/{id}/salary-structure`) — creating a new structure automatically supersedes the previous open-ended one (`EffectiveTo` set the day before the new `EffectiveFrom`), `409 SALARY_STRUCTURE_OVERLAP` if the new date isn't after the current one
|
||||
- [x] EmployeeLoan (+Installments) ledger (`GET/POST /employees/{id}/loans`) — creating a loan generates its full installment schedule up front; `IEmployeeLoanService.GetDueInstallmentsAsync` is what `PayrollCalculationService` consumes
|
||||
- [x] PayrollStatutorySetting (`/payroll-statutory-settings`), TaxSlab (`/tax-slabs`) — both effective-dated; creating a new statutory setting supersedes the prior open-ended one; tax slab creation validates no gap/overlap for the same effective date (`422 TAX_SLAB_GAP_INVALID`)
|
||||
- [x] `PayrollCalculationService` — Gross = Basic + allowance lines + OT; Net = Gross − Late − NoPay − Loan − EPF(employee) − Tax − OtherDeductions; **EPF-employer/ETF are informational-only, never subtracted**, matching the spec's "Company Contribution" framing; Tax via standard ascending marginal-slab computation over taxable earnings
|
||||
- [x] PayrollRun/PayrollLine/PayrollLineComponent + Draft→Approved→Locked workflow (`/payroll-runs`, generate/approve/lock/unlock/generate-payslips) — Generate blocked (`422 ATTENDANCE_NOT_CONFIRMED`) if any attendance batch for the period is still Draft/Validated; loan-installment (Pending→Deducted, balance decremented) and attendance-batch (Confirmed→UsedInPayroll) stamping deferred to **Lock**, not Generate/Approve, per docs A.4
|
||||
- [x] Unlock (`POST .../unlock`, mandatory reason) — reverses both the loan-installment and attendance-batch stamps made at Lock, back to Approved
|
||||
- [x] Payslip generation (Locked-only, idempotent) + HTML print view (`GET /payslips/{id}/view`) — no PDF dependency, per the confirmed decision
|
||||
|
||||
**Deviations / open items (recorded, not silently assumed):**
|
||||
- **The exact APIT taxable-income base is a real compliance question, not resolved here** (docs/12-BACKEND-HRM.md B.4 flags this explicitly) — `PayrollCalculationService` computes taxable income as `Basic + taxable allowance lines + Overtime` and applies the configurable `TaxSlab` table as a standard ascending marginal calculation; whether EPF-employee should reduce taxable income first, or whether OT should be taxable at all, needs finance/statutory sign-off before go-live.
|
||||
- **NoPayAmount currently only counts plain `Absent` days**, not unpaid-leave days — `AttendanceRecord` doesn't yet carry which `LeaveType` covered an `OnLeave` day (or whether it's paid), so all `OnLeave` days are currently treated as paid. A follow-up should either stamp the record with the leave's paid/unpaid flag at attendance-computation time, or join back to `LeaveRequest`/`LeaveType` during payroll calculation.
|
||||
- **OT/Late per-minute rate model**: `dailyRate = Basic ÷ daysInMonth`, `perMinuteRate = dailyRate ÷ WorkShift.StandardWorkingMinutes` — a simplification flagged in `12-BACKEND-HRM.md B.4` (tiered late/OT policies are a future improvement, not built now).
|
||||
- **`PayrollRunService.GenerateAsync` skips employees with no effective salary structure** for the period rather than failing the whole run — intentional (a partially-onboarded workforce shouldn't block payroll for everyone else), but means a run's employee count can silently be less than total active headcount; worth surfacing in the frontend as a warning list.
|
||||
- **Loan-installment/attendance-batch reversal on Unlock re-derives "what this run touched" by period/status query**, not from a stored per-run link table (e.g. any `Confirmed`-turned-`UsedInPayroll` batch for the run's period, any installment whose `PayrollRunId` matches). This is correct for the common case but would need a real link if multiple concurrent runs ever target overlapping periods/branches — not expected in this phase (one run per period/branch).
|
||||
|
||||
## 10. Sub-phase 2.4 — Reports
|
||||
> **Code complete (2026-07-23).** `dotnet build` clean (0 errors). No new entities/migration — pure read-only aggregation over Attendance/Payroll/Leave/Document tables (`IHrReportService`/`HrReportsController`, `/reports/hrm/*`), same posture as `StockController`'s on-hand/ledger queries. Runtime smoke-test not yet run.
|
||||
- [x] Attendance summary (`GET /reports/hrm/attendance-summary?periodYear=&periodMonth=&departmentId=`), OT report (`.../overtime`), late-arrival report (`.../late-arrivals`)
|
||||
- [x] Payroll register (`.../payroll-register?payrollRunId=`) — `TotalDeductions` computed as `GrossSalary − NetSalary` (informational EPF-employer/ETF already excluded since they were never subtracted from Net)
|
||||
- [x] Employee salary history (`.../salary-history?employeeId=`) — full `EmployeeSalaryStructure` revision history, ordered newest first
|
||||
- [x] Leave balance report (`.../leave-balances?year=`), document expiry report (`.../document-expiry?withinDays=`)
|
||||
|
||||
## Done
|
||||
<!-- move [x] items here with date + note if the active list grows long -->
|
||||
|
||||
|
||||
Reference in New Issue
Block a user