Project

General

Profile

Actions

Feature #140

open
SV SV

[HR][P2] Pay and balance correctness

Feature #140: [HR][P2] Pay and balance correctness

Added by Sravan V S about 2 months ago. Updated 3 days ago.

Status:
Resolved
Priority:
Urgent
Assignee:
Target version:
-
Start date:
09/28/2026
Due date:
% Done:

0%

Estimated time:
Resolved By:

Description

Leave workflow: guard-before-deduct in one commit, reject guard, reserve on ManagerApproved, concurrency token, reviewer from JWT, self-approval block. One leave-day counter (holidays, weekly off); ESS balance from the ledger. Attendance time base: convert regularization rows to one convention, hospital-local times everywhere, one-off data fix for already-regularized rows. Overnight shifts, OT break, future dates, per-date credential check at assign/publish/OPD-sync. Day boundaries on credentials, onboarding, training, FY. Also covers the ESS "next 7 days" previous-day-shift bug (A-34/B-26, same root cause, fix once).

Size: L

Needs owner decision #13 (leave-year part only) (see #137).

Source: clinq-docs/modules/hr/reviews/2026-09-28-ui-audit/ (branch docs/hr-ui-audit-2026-09-28)

Findings covered:

  • C-01
  • C-03
  • C-04
  • C-12
  • C-13
  • C-06
  • C-07
  • C-46
  • C-14
  • C-47
  • B-20
  • B-04
  • B-12
  • B-23
  • B-41
  • B-11
  • B-13
  • B-15
  • B-35
  • D-24
  • D-40
  • D-30
  • D-17
  • A-34
  • B-26

SV Updated by Sravan V S about 2 months ago Actions #1

  • Assignee set to Sravan V S

S- Updated by SK - 13 days ago Actions #2

  • Assignee changed from Sravan V S to SK -

S- Updated by SK - 13 days ago Actions #3

  • Status changed from New to Resolved
  • Assignee changed from SK - to Roshni Harish.
  • Resolved By set to SK -

Fixed and merged to dev on nuvita-hr. Worked from the ticket's terse description, verified each item against current code first (source audit doc confirmed permanently lost, see #137). Owner decision #13 (leave-year boundary) deliberately untouched everywhere - kept the pre-existing April-March convention already in the code, per your confirmed answer on #137.

Leave workflow - all confirmed real, all fixed in one transaction per operation:

  • Guard-before-deduct: balance deduction and the approval status save used to happen in two separate commits with a window between them - now one transaction.
  • Reserve on ManagerApproved: didn't exist - added (a new Reserved column on the balance ledger), so two overlapping approvals can't both spend the same balance.
  • Concurrency token: missing - added via the existing ModifiedAt column as a concurrency token (no new column needed); verified with a real dual-connection race test.
  • Reviewer from JWT: confirmed a real spoofing hole - the reviewer identity was a client-supplied field in the request body on four endpoints. Now always resolved from the authenticated caller.
  • Self-approval block: missing - added.
  • One leave-day counter: confirmed a real double-charge bug - leave spanning a public holiday was charged for the holiday AND a leave day, since the counter excluded roster off-days but not holidays. Fixed.
  • ESS balance from the ledger: confirmed real drift - the self-service balance was computed independently from the one the approval path actually uses, ignoring carry-forward/encashed/reserved amounts. Now reads the same ledger.
  • Flagged, not fixed (same drift class, not in this ticket's named scope): a third, independently-computed leave balance on the admin employee-detail page has the identical risk.

Attendance time base: confirmed a real timezone bug - approved regularizations stored the employee's requested IST time by relabeling it UTC rather than converting it, landing 5.5 hours off from every other attendance record. Fixed going forward, plus a one-off data-fix migration for already-approved rows with the wrong time. This migration needs a human to validate the matched-row count against a copy of real tenant data before this ships to any real tenant - it's a heuristic match (no FK links a regularized row back to its regularization), full validation steps are in the migration's own code comments.

Overnight shifts / OT / future dates / credentials: overnight shift duration was already correct (fixed previously). Confirmed and fixed: phantom overtime from a break-duration miscalculation; no guard against a future-dated manual punch; roster assignment and the OPD scheduling sync both checked a credential's status "as of now" instead of as of the actual roster date (the OPD sync endpoint had no credential check at all before this). Flagged, not fixed: the roster-publish completeness gate still checks "today" uniformly rather than per-entry-date - making that per-date is a product call (does one blocked day shrink a slot, or block the whole publish) that needs a decision, not a mechanical fix.

Day boundaries: confirmed and fixed the same "wrong timezone for today" bug in Onboarding, Training (including CME financial-year calculations), and the Dashboard - all now use the #138 hospital-day clock helper. Confirmed financial-year labeling itself has no bug (no "today" involved there). Flagged, not investigated: the payroll calculation engine has its own raw UtcNow parameter - judged too large and unaudited for this pass.

ESS "next 7 days" bug (A-34/B-26): confirmed the exact root cause #138 had already flagged and left as a known PENDING item - a browser-local-midnight bug stacked with a UTC-conversion bug in the dashboard's 7-day shift strip, which could show the previous day's shift right after midnight IST. Rewritten properly using the #138 clock helper; the stale PENDING entry removed from the guard.

Tests: HR backend unit 1243/1243 passed; HR backend integration 250/250 passed; HR frontend 105/105 files, 809/809 tests passed, type-check clean.

Deploy gate: the ordinary schema migration (AddLeaveBalanceReservation) is safe to run normally. The one-off data-fix migration (RepairMisconvertedRegularizedAttendanceTimes) needs sign-off per the validation steps above before running against any real tenant.

S- Updated by SK - 13 days ago Actions #4

  • Assignee changed from Roshni Harish. to Sravan V S

S- Updated by SK - 12 days ago Actions #5

  • Assignee changed from Sravan V S to Roshni Harish.

Double-checked directly against current dev code (commit 3228600, build + targeted unit/integration tests all green) before reassigning - every claim in the resolution note checks out in the actual files:

  • Reserve on ManagerApproved: LeaveBalance.cs has a real Reserved column with Reserve/ReleaseReservation/ConvertReservationToUsed methods.
  • Concurrency token: LeaveBalanceConfiguration.cs - builder.Property(b => b.ModifiedAt).IsConcurrencyToken().
  • Reviewer from JWT: ApproveLeaveRequest/RejectLeaveRequest/etc. DTOs now carry only ReviewNotes (no client-supplied reviewer field), and LeaveRequestService.ApproveAsync resolves reviewedBy from _httpContextHelper.GetCurrentUserOrSystem().
  • Self-approval block: EnsureReviewerIsNotTheRequestingEmployeeAsync is called at the top of Approve/Reject/ManagerApprove.
  • One leave-day counter: rosterOffDates.Union(holidayDates).Count() - holiday double-charge fixed.
  • Attendance timezone + per-date credential check: already independently verified in #151/#153.

This is genuinely done, not just claimed. Reassigning to Roshni for QA sign-off (the leave-year decision #13 was deliberately NOT touched by this ticket - that's #146, verified separately).

RH Updated by Roshni Harish. 3 days ago Actions #6

  • Assignee changed from Roshni Harish. to Sravan V S
Actions

Also available in: PDF Atom