# ESS Leave Module — Deep Analysis **Date:** 2026-03-04 **Module path:** `projects/ess/transaction/leave/` **Services path:** `projects/ess/transaction/service/` --- ## File Inventory | File | Lines | Role | |------|-------|------| | `leaverequest/leaverequest.component.ts` | 2226 | Leave request form — submit new / edit leave | | `leaverequest/leaverequest.component.html` | 246 | Template (with heavy commented-out blocks) | | `leaverequest/leaverequest.component.scss` | ~380 | Styles | | `leaverequestlist/leaverequestlist.component.ts` | 1222 | List view — summary, history, D3 chart | | `leaverequestlist/leaverequestlist.component.html` | 222 | Template | | `leaverequestlist/leaverequestlist.component.scss` | ~365 | Styles | | `service/leaverequest.service.ts` | 126 | Biz service layer (form load/save/delete + HTTP wrappers) | | `service/leaverequestlist.service.ts` | 130 | Same pattern for list component | | `*.spec.ts` (both) | 1 each | Empty — zero test coverage | --- ## Issues Summary | Priority | Category | Count | |----------|----------|-------| | P0 | Security | 5 | | P0 | Missing OnPush | 2 | | P0 | Memory — untracked setTimeout | 13 | | P0 | Subscriptions without takeUntil | 2 | | P0 | console.log in production | 7 | | P1 | `any` types throughout | 30+ | | P1 | Magic number module IDs | 25+ | | P1 | Hardcoded English strings | 50+ | | P1 | Duplicate `(PicklistValue)` handlers | 8 | | P1 | Non-responsive dialog widths | 5 | | P1 | Missing `trackBy` on `*ngFor` | 3 | | P1 | Repeated API calls (not combined) | 4 | | P2 | Hardcoded fixed dimensions | 6 | | P2 | Inconsistent date parsing | 3 patterns | | P2 | Dead commented-out code | ~100 lines | | P2 | No RTL CSS | Both files | | P2 | Zero unit tests | All spec files | | P2 | Financial year hardcoded (2025–2026) | 1 | | Functional | Leave balance check only for CL/COFF | Bug | | Functional | `Reportdetailservice` unguarded `.responseValue[0]` | Crash risk | | Functional | `GetEmployeeThumbnail` result unused | Dead code | | Functional | `selectDate('today')` called twice on init | Duplicate call | | Functional | OperationType strings vs numbers mixed | API inconsistency | --- ## P0 — Security ### 1. LoginDTO from sessionStorage **Files:** - `leaverequest.component.ts:91` — constructor - `leaverequestlist.component.ts:99` — constructor - `leaverequest.service.ts:27` — constructor - `leaverequestlist.service.ts:27` — constructor ```typescript // Current (P0 — XSS-vulnerable) this.LoginDTO = JSON.parse(sessionStorage.getItem('LoginDTO') as any) // Fix: inject auth service and read from signal private auth = inject(AuthService); get loginDto() { return this.auth.loginDto(); } ``` SessionStorage is accessible to any JavaScript on the page (XSS). Sensitive auth data must come from an httpOnly cookie or a properly injected auth signal. ### 2. Additional sessionStorage reads `leaverequest.component.ts:114–134` reads 5 keys from sessionStorage for cross-page state passing: - `selectedCalendarDate` - `hideListButton` - `selectedEmployeeId`, `selectedEmployeeCode`, `selectedEmployeeName` `leaverequestlist.component.ts:106` reads `navigatedFromCalendar`. **Fix:** Pass navigation state via Angular Router state (`extras.state`) or a shared signals service (`DataPassingService`). Never sessionStorage. --- ## P0 — Missing ChangeDetectionStrategy.OnPush Both components are decorated without `changeDetection`: ```typescript // leaverequest.component.ts:16–21 @Component({ selector: 'gb-leaverequest', ... // ← NO changeDetection: ChangeDetectionStrategy.OnPush }) // leaverequestlist.component.ts:23–28 @Component({ selector: 'gb-leaverequestlist', ... // ← NO changeDetection: ChangeDetectionStrategy.OnPush }) ``` `LeaveRequestComponent` is 2226 lines with multiple grid updates, 13 `setTimeout` calls, and is embedded in the attendance calendar. Default CD fires on every event globally. **This is a P0 perf bomb.** `LeaveRequestListComponent` renders a D3 chart + two tables. Default CD causes chart redraws on every keystroke anywhere in the app. --- ## P0 — Untracked setTimeout (13 instances) ### LeaveRequestComponent | Line | Delay | Purpose | |------|-------|---------| | 135 | 100ms | Calls `Framework.User.GetUser` API + form patch | | 237 | 100ms | Calls `BiztransactionService(true)` | | 533 | 0ms | Updates grid array + form patch | | 1040 | 0ms | Sets new grid data from API response | | 1077 | 100ms | Clear form after dialog | | 1321 | 100ms | Grid update after toggle change | | 1397 | 0ms | Grid update | | 1590 | 0ms | Grid update | | 1636 | 100ms | AddNew → calls `BiztransactionService` | | 1642 | 200ms | AddNew → calls `Framework.User.GetUser` API | | 2207 | 100ms | AddNew → patch default grid values | ### LeaveRequestListComponent | Line | Delay | Purpose | |------|-------|---------| | 126 | 100ms | ESS employee init + form patch | | 295 | 100ms | `drawEmptyChart()` | | 1065 | 100ms | `drawChart()` — called on every employee switch | **None of these are tracked (`clearTimeout`) or cleared in `ngOnDestroy`.** The nested setTimeout at lines 1636+1642 (100ms outer, 200ms inner during AddNew) creates a timing race condition: if the user navigates away before 200ms elapses, the inner callback fires on a destroyed component. **Fix pattern:** ```typescript private timeouts: ReturnType[] = []; // Instead of: setTimeout(() => { ... }, 100); // Use: this.timeouts.push(setTimeout(() => { ... }, 100)); ngOnDestroy() { this.timeouts.forEach(clearTimeout); this.destroy$.next(); this.destroy$.complete(); } ``` --- ## P0 — Subscriptions Without takeUntil ### `BiztransactionService()` — leaverequest.component.ts:284–298 ```typescript // Line 286 — NO takeUntil this.service.BizTransactionClassService(this.MenuRights.BizTransactionClassId) .subscribe((BizTransactionSelectlist: any) => { // Line 290 — nested, NO takeUntil this.service.BizTransactionType(BizTransactionSelectlist.responseValue[0].Id) .subscribe((response: any) => { ... }); }); ``` This is called on init AND on every AddNew. Each call adds a permanently active subscription on a root-provided singleton service. **This will stack indefinitely.** ### `Reportdetailservice()` — leaverequestlist.component.ts:179 ```typescript this.service.Reportdetailservice(-1399998365).subscribe((menudetails: any) => { // No takeUntil this.MenuDetail = menudetails.responseValue; this.loadLeaveHistoryDirect(this.UserId); // Triggers another HTTP call }); ``` --- ## P0 — console.log in Production | File | Line | Content | |------|------|---------| | `leaverequest.component.ts` | 92 | `"this.LoginDTO"` — logs full auth object | | `leaverequest.component.ts` | 101 | `"jsonname"` | | `leaverequest.component.ts` | 944 | `"apiresponse for grid data:"` | | `leaverequestlist.component.ts` | 100 | `"this.LoginDTO"` — logs full auth object | | `leaverequestlist.component.ts` | 158–159 | Work period dates | | `leaverequestlist.component.ts` | 594 | `"Graphresponse"` + full chart response | Lines 92 and 100 log the entire LoginDTO to the browser console — this includes UserId, UserCode, WorkOUId and potentially other sensitive fields. **Fix:** Replace all with `GbConsoleService`. Remove login-related logs entirely. --- ## P1 — Duplicate (PicklistValue) Event Handlers Angular processes only the **last** binding of the same output on an element. The first is silently ignored. ### leaverequest.component.html ```html ``` ### leaverequestlist.component.html All 8 picklist bindings have the same pattern (lines 27, 30, 37, 40, 47, 50, 57, 60): ```html ``` Wait — in the list template the order is reversed. `OnPicklistChange` is listed second, so `GetEMployee` is dead. This means **employee selection does not trigger data loading** in most list picklists. **Fix:** Combine the two handlers: ```typescript onEmployeePicklistChange(event: any) { this.OnPicklistChange(event); this.GetEMployee(event); } ``` ```html ``` --- ## P1 — Magic Number Module IDs The following magic numbers appear 25+ times across both components: | Constant | Value | Meaning | |----------|-------|---------| | ERP_MODULE_ID | `-1499999788` | ERP leave module | | ESS_MODULE_ID | `-1399999915` | ESS leave module | | CALENDAR_MENU_ID | `-1399986890` | Calendar navigation into leave | | ENTITY_ID | `-1399999935` | Employee entity type | | BIZ_CLASS_ID | `-1399999955` | Leave biz transaction class | | REPORT_MENU_ID | `-1399998365` | Leave report menu | **Fix:** Define an enum or constants file: ```typescript // leave.constants.ts export const LeaveModuleIds = { ERP: -1499999788, ESS: -1399999915, CALENDAR_MENU: -1399986890, EMPLOYEE_ENTITY: -1399999935, BIZ_CLASS: -1399999955, REPORT_MENU: -1399998365, } as const; ``` --- ## P1 — API Call Optimization / Redundant Calls ### Repeated `Framework.User.GetUser` calls `Framework.User.GetUser` is called: 1. During `ngOnInit` (inside setTimeout at line 135) — `leaverequest.component.ts` 2. On `AddNew` (inside nested setTimeout at line 1642) — same component 3. On `loadEmployeeData()` — `leaverequestlist.component.ts:418` 4. On `updateUIFromFilter()` — `leaverequestlist.component.ts:921` 5. On `getESSEmployee()` — `leaverequestlist.component.ts:345` In LeaveRequestListComponent, `getESSEmployee()` calls `GetEMployee()` which calls `loadEmployeeData()` which calls `GetBizTransaction(Framework.User.GetUser)` again — **two User.GetUser calls per employee selection**. **Fix:** Cache the user details on first load in a signal. The auth service already has this data via LoginDTO. ### Repeated Employee EMPLOYEE criteria calls `/cs/Criteria.svc/List/?ObjectCode=EMPLOYEE` is called: 1. `loadEmployeeDetails(userId)` — on init 2. `loadEmployeeData()` — on employee picklist change (calls it again internally) 3. `onNavigateToList()` — when navigating back from form to list All three build identical criteria objects and make the same HTTP request. Result is never cached. **New API suggestion:** A combined endpoint `GET /prs/Leave.svc/EmployeeLeaveContext/?EmployeeId={id}&OUId={ouId}&PeriodFrom={from}&PeriodTo={to}` that returns in a single call: - Employee details (Department, Designation, WorkOUId) - Leave balance summary (currently: `/prs/Leave.svc/LeaveStatusReport/`) - Leave history (currently: `/prs/TLeave.svc/TLeave/`) - Monthly chart data (currently: `/prs/TLeave.svc/TotalLeaveTaken/`) This replaces **4 sequential HTTP calls** with **1 call** whenever an employee is selected or the list is loaded. ### Sequential HTTP waterfall in `loadEmployeeData()` ``` GetBizTransaction(User.GetUser) → callback: GetLeaveRequest(EMPLOYEE criteria) → callback: loadDailyLeaveReportForEmployee() → 3rd HTTP call loadLeaveChartDataForEmployee() → 4th HTTP call loadLeaveHistoryForEmployee() → 5th HTTP call ``` 5 sequential HTTP calls = **5× round trip latency**. With forkJoin/combineLatest, calls 3–5 can run in parallel after call 1–2 complete. ### `Reportdetailservice(-1399998365)` followed by `loadLeaveHistoryDirect()` In `leaverequestlist.component.ts:179–187`: ```typescript this.service.Reportdetailservice(-1399998365).subscribe((menudetails: any) => { // Uses only MenuDetail[0].PeriodType etc — not actually used for anything downstream this.loadLeaveHistoryDirect(this.UserId); // This doesn't even use MenuDetail }); ``` `loadLeaveHistoryDirect` builds its own hardcoded criteria and does not use any data from `Reportdetailservice`. These can run in parallel with `forkJoin`. --- ## P1 — Missing trackBy on *ngFor | Template | Line | Array | Performance risk | |----------|------|-------|-----------------| | `leaverequest.html` | 166 | `Dailyreport` | Re-creates all DOM rows on every CD | | `leaverequestlist.html` | 108 | `Dailyreport` | Same | | `leaverequestlist.html` | 175 | `LeaveReport` | Same — full leave history list | ```typescript trackByLeaveId(index: number, item: any): number { return item.LeaveId ?? item.TLeaveId ?? index; } ``` --- ## P1 — Non-Responsive Dialog Widths | Location | Width | Should be | |----------|-------|-----------| | `leaverequest.component.ts:1143` | `350px` | `min(350px, 95vw)` | | `leaverequest.component.ts:1070` | `400px` | `min(400px, 95vw)` | | `leaverequest.component.ts` (multiple) | `400px` | `min(400px, 95vw)` | | `leaverequest.service.ts:90` | `600px` | `min(600px, 95vw)` | | `leaverequestlist.service.ts:71` | `600px` | `min(600px, 95vw)` | --- ## P1 — Hardcoded English Strings (No Transloco) Full list of hardcoded UI strings: **leaverequest.component.html:** - Button labels: `Yesterday`, `Today`, `Tomorrow` (lines 137–147) - Column headers: `Type`, `Balance` (lines 163–164) **leaverequestlist.component.html:** - Section titles: `Leave/Onduty Summary`, `Leave/Onduty Summary-Graph`, `Leave/Onduty History` (lines 74, 148, 158) - Table headers: `Entry Date`, `Leave/Onduty Req. #`, `Type`, `From`, `To`, `# Days`, `Status`, `Remarks` (lines 164–171) - Empty state: `No leave records found.` (lines 135, 208) - Status labels in template: `Approved`, `Pending`, `Rejected` (inline ngSwitch) **leaverequest.component.ts:** - `leaveDayTypeArray` values: `FullDay`, `FirstHalf`, `SecondHalf` (lines 56–60) - Dialog message at line 1145: `Pending Leaves are not enough for: ${element.LeaveName}` - `weekday` array at line 1239: `["Sunday", "Monday", ...]` **leaverequestlist.component.ts:** - Leave status map (line 72–79): `'Pending'`, `'Active/Approved'`, `'Deleted'`, etc. - `monthOrder` array (lines 314, 1008, 1031) **leaverequestlist.service.ts:** - Line 55: `this.translate.translate('' + jsonvalue.Label) + " Should not be Empty"` — uses Transloco for label but concatenates untranslated ` Should not be Empty` suffix - Line 59: `"Please Provide Valid Percentge - "` — typo: "Percentge" All these should use `transloco` pipe in templates and `translocoService.translate()` in TypeScript. --- ## P1 — `any` Types Both components declare almost all properties as `any`. Critical ones: **leaverequest.component.ts:** - `LoginDTO: any` — must be `ILoginDTO` - `BizType: any` — must be `'Auto' | 'Manual'` - `EmployeeCode/Name/Id: any` — must be `string/string/number` - All form event params: `event: any` — should be typed picklist event interface - `Dailyreport: any[]` — needs `ILeaveBalance[]` interface **leaverequestlist.component.ts:** - Declares `LoginDTODetail!: ILoginDTO` (line 40) but then also `LoginDTO: any` (line 43) — two variables for the same data, one typed one not **leaverequest.service.ts / leaverequestlist.service.ts:** - `GetUrl = GetUrl as any`, `SaveUrl = SaveUrl as any` — defeats type checking on all URLs - `LoginDTODetail: any` — should be `ILoginDTO` --- ## P2 — Hardcoded Dimensions | Location | Value | Issue | |----------|-------|-------| | `leaverequest.html:15` | `width: 920px` | Breaks on tablet/mobile | | `leaverequest.html:90` | `width: 720px; height: 164px` | Fixed | | `leaverequestlist.html:80` | table with no responsive width | Horizontal scroll on mobile | | `drawChart()` line 1074 | `width = 600 - ...` | Hardcoded SVG — doesn't resize | | `drawEmptyChart()` line 304 | `width = 600 - ...` | Same | | `leaverequestlist.scss` | `.historycontainer { width: 954px }` | Fixed container | The D3 chart should use `ResizeObserver` or read `element.clientWidth` to be responsive. --- ## P2 — Financial Year Hardcoded `leaverequestlist.component.ts:168–169`: ```typescript const financialYearStart = new Date(Date.UTC(2025, 3, 1, 0, 0, 0)); // April 2025 const financialYearEnd = new Date(Date.UTC(2026, 2, 31, 0, 0, 0)); // March 2026 ``` These dates are commented out below so don't currently affect logic — but the code will be wrong in FY 2026–27. Remove dead code and compute dynamically from current date. --- ## P2 — Inconsistent Date Parsing Three different patterns used for `.NET` date strings: ```typescript // Pattern 1 (most correct — leaverequest.component.ts:371) const match = dateString.match(/\/Date\((\d+)\)\//); const ms = parseInt(match[1], 10); // Pattern 2 (missing anchor — leaverequest.component.ts:578) const matchResult = epochString.match(/\d+/); // matches first number found // BUG: will match '123' in '/Date(123)/' but also matches '/' in other strings // Pattern 3 (manual replace — leaverequestlist.component.ts:148) parseInt(this.LoginDTO.WorkPeriodFromDate.replace('/Date(', '').replace(')/', ''), 10); ``` **Fix:** One utility function, one place: ```typescript export function parseDotNetDate(val: string): Date | null { const match = val?.match(/\/Date\((\d+)\)\//); return match ? new Date(parseInt(match[1], 10)) : null; } ``` --- ## P2 — Mixed OperationType Types (API Bug) In `loadLeaveChartDataForEmployee()` (leaverequestlist.component.ts:562): ```typescript { "FieldName": "OUId", "OperationType": "5", ... } // ← string "5" { "FieldName": "EmployeeId", "OperationType": "5", ... } // ← string "5" { "FieldName": "PeriodFromDate", "OperationType": 10, ... } // ← number 10 ``` Some fields use string `"5"` for OperationType, others use number `10`. The API may handle this through JSON coercion, but it's inconsistent and fragile. --- ## P2 — Dead Code 1. **~100 lines of commented-out code** in `leaverequest.component.ts` (lines 301–361, lines 939–979 comments) 2. `GetEmployeeThumbnail()` in `leaverequestlist.component.ts:400–410` — fetches thumbnail URL from API, assigns result to local `ans` variable, never uses it, never assigns to `this.UserThumbNail` 3. `ApplyFilter()` method in `leaverequestlist.component.ts` — entire method is commented out (lines 767–911); the filter button is also hidden in template (line 15 commented) 4. `updateUIFromFilter()` — exists but is never called (ApplyFilter which called it is dead) 5. `leaveDayTypeArray` in `leaverequest.component.ts:56–60` — array defined but never used in any template or method --- ## Functional Issues ### 1. Leave balance check only for CL / COFF `checkLeaveBalanceFromDailyReport()` at line 1138: ```typescript if (element.LeaveCode === selectedLeaveCode && (selectedLeaveCode === 'CL' || selectedLeaveCode === 'COFF')) { ``` Balance validation only runs for Casual Leave and Comp-Off. All other leave types (EL, SL, ML, etc.) can be applied regardless of balance. **Fix:** Remove the `CL/COFF` filter. Check balance for any leave type that has `LeaveIsBalanceMaintained === true`. ### 2. `Reportdetailservice` crash on empty response `leaverequestlist.component.ts:180–181`: ```typescript this.MenuDetail = menudetails.responseValue; this.PeriodType = this.MenuDetail[0].PeriodType; // ← crashes if empty ``` No null/length check before accessing `[0]`. ### 3. `TLeaveRetrivalFunction` crash on empty TLeaveDetailArray `leaverequest.component.ts:1176`: ```typescript this.UserThumbNail = LeaveRequestList.responseValue.TLeaveDetailArray[0].EmployeeThumbNail || ""; ``` No null check on `TLeaveDetailArray` or its first element. Will throw if server returns an empty array. ### 4. `selectDate('today')` called twice on init `leaverequest.component.ts:107` calls `selectDate('today', true)` unconditionally, then the `else` branch at line 120 may call it again. If `msDateFormat` is not set (normal case), `selectDate('today')` fires twice, triggering two `getWeekdaysBetween` + `getDatesBetween` calculations. ### 5. `ngOnInit` reads sessionStorage keys then immediately removes them `leaverequest.component.ts:117`: `sessionStorage.removeItem('selectedCalendarDate')` is called immediately after reading. If any Angular error occurs after this line but before the values are applied, the state is lost permanently. Fix: remove items only after successfully applying them. ### 6. Picklist `OnPicklistChanges` ESS branch resets to LoginDTO.UserId on clear `leaverequest.component.ts:743`: ```typescript if ((this.MenuRights as any).ModuleId == -1399999915 ...) { this.form.get(Event.Field.LinkId)?.patchValue(this.LoginDTO.UserId); } ``` When a manager clears an employee picklist in ESS Admin mode, EmployeeId resets to the manager's own ID instead of -1. This would cause subsequent saves to submit leave for the wrong person. ### 7. Leave balance dialog doesn't prevent save `checkLeaveBalanceFromDailyReport()` shows a dialog when balance is 0 and clears the leave type name field. But it does **not** prevent the form from saving — if the user manually re-enters a leave type ID through another path, save proceeds with 0 balance. The validation should be part of the save guard. --- ## Architecture Recommendations ### 1. Introduce `LeaveContextService` Both components duplicate the same employee context logic (load user, load employee details, load daily report, load chart). Extract to a shared signals-based service: ```typescript @Injectable() export class LeaveContextService { private employeeId = signal(-1); employeeDetails = rxResource({ request: () => ({ id: this.employeeId() }), loader: ({ request }) => this.loadEmployeeDetails(request.id) }); leaveBalance = rxResource({ request: () => ({ id: this.employeeId(), ouId: this.auth.loginDto().WorkOUId }), loader: ({ request }) => this.loadLeaveBalance(request) }); leaveHistory = rxResource({ request: () => ({ id: this.employeeId() }), loader: ({ request }) => this.loadLeaveHistory(request.id) }); setEmployee(id: number) { this.employeeId.set(id); } } ``` ### 2. Cross-page navigation via Router state, not sessionStorage ```typescript // Caller (calendar component): this.router.navigate(['/leave/request'], { state: { date: selectedDate, employeeId: empId, hideList: true } }); // Receiver (leaverequest): const nav = this.router.getCurrentNavigation()?.extras.state; this.msDateFormat = nav?.['date']; ``` ### 3. D3 chart should be a standalone component `drawChart()` is 90 lines of D3 code inside `LeaveRequestListComponent`. Extract to `LeaveBarChartComponent` with `@Input() data: ChartDataPoint[]`. ### 4. Replace nested setTimeout with proper Angular lifecycle Most timeouts exist to defer until the form is rendered. Use `afterRender()` (Angular 17+) or `ngAfterViewInit` for post-render work: ```typescript ngAfterViewInit() { // Instead of setTimeout(() => { this.BiztransactionService(); }, 100) this.BiztransactionService(); } ``` --- ## Suggested New API Endpoints ### Combined employee leave context ``` POST /prs/Leave.svc/EmployeeLeaveContext/ Body: { EmployeeId, OUId, PeriodFrom, PeriodTo } Returns: { Employee: { Dept, Designation, WorkOUId }, LeaveBalance: [...], MonthlyTaken: [...], LeaveHistory: [...] } ``` Eliminates 4 sequential calls on every employee switch. ### Combined leave detail on leave type select ``` GET /prs/Leave.svc/LeaveApplyContext/?LeaveId={id}&EmployeeId={empId}&From={ts}&To={ts} Returns: { Leave: { LeaveCode, LeaveName, LeaveIsBalanceMaintained }, DayTypes: [...], // what DailyAttendance API currently returns CurrentBalance: number } ``` Eliminates the two sequential calls when a leave type is picked: 1. `/prs/Leave.svc/?LeaveId=` 2. `/prs/DailyAttendance.svc/Get/DayType/Before/Applying/Levae/TimeSlip/` Note: The current URL has a typo: "Levae" instead of "Leave". --- ## Prioritized Fix List ### Do immediately (P0) 1. Add `ChangeDetectionStrategy.OnPush` to both components 2. Add `takeUntil(this.destroy$)` to `BiztransactionService` nested subscribes (lines 286, 290) and `Reportdetailservice` (line 179) 3. Track all `setTimeout` handles, clear in `ngOnDestroy` 4. Remove all `console.log` — especially lines 92 and 100 that log LoginDTO 5. Replace sessionStorage navigation state with Router state or `DataPassingService` signals ### High priority (P1) 6. Fix duplicate `(PicklistValue)` handlers in templates — employee selection is broken in list view 7. Add null checks in `Reportdetailservice` callback (line 180) and `TLeaveRetrivalFunction` (line 1176) 8. Define `LeaveModuleIds` constants enum — eliminate 25+ magic numbers 9. Add `trackBy` to all `*ngFor` 10. Fix leave balance check to cover all leave types, not just CL/COFF 11. Fix `OnPicklistChanges` ESS clear logic resetting to wrong userId ### Medium (P2) 12. Consolidate date parsing into one utility function 13. Make D3 chart dimensions responsive 14. Add Transloco keys for all UI strings 15. Remove dead code (~100 lines comments + `GetEmployeeThumbnail` + `ApplyFilter` + `leaveDayTypeArray`) 16. Fix OperationType string/number inconsistency in chart criteria 17. Write unit tests for service layer and core methods --- *Analysis by Claude Code — 2026-03-04*