# Encash Request — Deep Analysis

**Files analyzed:**
- `projects/ess/transaction/encashrequest/encashrequest.component.ts` (513 lines)
- `projects/ess/service/encashrequest.service.ts` (140 lines)
- `projects/ess/dbservice/encashrequest.db.service.ts` (referenced)

**Date:** 2026-03-04

---

## Summary Scorecard

| Category | Issues | P0 | P1 |
|----------|--------|----|-----|
| Security | 2 | 2 | 0 |
| Functional Bugs | 7 | 3 | 4 |
| Memory Leaks | 3 | 0 | 3 |
| Performance | 1 | 0 | 1 |
| Code Quality | 8 | 0 | 4 |
| API / Service | 3 | 1 | 2 |

---

## P0 — Critical Issues

### SEC-01: `sessionStorage.getItem('LoginDTO')` in Constructor

`encashrequest.component.ts:73` and `encashrequest.service.ts:28`:
```typescript
// constructor:
this.loginDTO = JSON.parse(sessionStorage.getItem('LoginDTO') as any);
// service constructor:
this.LoginDTODetail = JSON.parse(sessionStorage.getItem('LoginDTO') as any);
```
**Fix:** Use `GbAppStateService` or `GbConfigService` signal.

---

### FUNC-01: HTTP Call in Constructor (`RateandLeaveBalanceshowing`)

`encashrequest.component.ts:81`:
```typescript
constructor(...) {
    ...
    this.RateandLeaveBalanceshowing(this.userid)  // HTTP call in constructor
}
```
HTTP calls must not be in constructors — Angular DI may not be fully initialized, and the component may be constructed without being rendered (e.g., in tests or lazy loading). If the call fails, it silently swallows the error.

**Fix:** Move to `ngOnInit()`.

---

### FUNC-02: `OnpicklistLoad` Loads Data but Never Patches the Form

`encashrequest.component.ts` lines 203-224:
```typescript
public OnpicklistLoad(Event: any) {
    if (Event.SelectedId != 0 && Event.SelectedId != -1) {
        this.service.formloadservice('encashrequest', Event.SelectedId)
            .subscribe((encashrequestData: any) => {
                let data = encashrequestData.responseValue
                data.PayProductionDetailArray.forEach(...)  // processes array
                // ❌ form.patchValue() is never called
            });
        this.MinimumBalanceRequired = false
        this.AvailableLeavebalance = false
        this.AvailableforEncashment = false
    }
}
```
When viewing an existing encash record, the HTTP response is received and processed (the array is re-mapped) but `this.form.patchValue(data)` is never called. The form always shows empty fields in edit mode. This is a functional regression.

**Fix:** Add `this.form.patchValue(data)` after the array processing.

---

### FUNC-03: `cdr.detectChanges` Called Without Parentheses — No-Op

`encashrequest.component.ts:477`:
```typescript
this.cdr.detectChanges    // ❌ missing () — this is a property access, not a method call
```
This is a silent no-op. The change detection that was intended here never fires. Since the component has `OnPush`, any UI update that depends on this call will not render.

**Fix:** `this.cdr.detectChanges()`

---

### FUNC-04: `MBRId` and `AEncashId` Assigned the Same Field — Wrong Values Shown

`encashrequest.component.ts:194-195`:
```typescript
let MBRId = data[0].EncashEligibleDays    // MinimumBalanceRequired field
let AEncashId = data[0].EncashEligibleDays // AvailableForEncashment field — SAME SOURCE
```
Both "Minimum Balance Required" and "Available for Encashment" display `EncashEligibleDays`. These should come from different API fields (e.g., `MinimumBalanceRequired` and `AvailableForEncashment`). The user always sees the same value in both fields regardless of actual balance.

---

## P1 — High Priority Issues

### FUNC-05: `checkApplyEn()` — Variable Shadowing with `var`

`encashrequest.component.ts:296-317`:
```typescript
public checkApplyEn() {
    var checkDifference = 0;    // outer declaration (never used — always overwritten)

    if (availBalCheckRaw != 0 && availBalCheckRaw >= minBalCheck) {
        var checkDifference = parseFloat(availBalCheckRaw) - parseFloat(minBalCheck);
    } else {
        var checkDifference = 0;
    }

    if (checkDifference > 0) { checkdiffval = checkDifference }
    // ...
    if (checkDifference == undefined) { checkDifference = 0 }  // ← can NEVER be true
```
- `var` hoisting means three declarations of the same variable — confusing, not valid in strict mode
- `if (checkDifference == undefined)` can never be true because the if/else above always assigns it
- The outer `var checkDifference = 0` at line 296 is immediately shadowed and meaningless

**Fix:** Use `const`/`let`, single declaration, remove the impossible `undefined` check.

---

### FUNC-06: `calculateDateDifference` Reads Wrong Fields

```typescript
public calculateDateDifference(event: any) {
    const date1 = this.getDateFromEpochString(this.form.get('InsuranceStartDate')?.value);
    const date2 = this.getDateFromEpochString(this.form.get('InsuranceEndDate')?.value);
```
This is the **encash request** form but it reads `InsuranceStartDate` / `InsuranceEndDate` — insurance form fields that don't exist in the encash request form. These will always be `null`, meaning the validation never fires. This is dead validation code carried over from another module.

**Fix:** Use the actual request date fields from the encash form JSON, or remove if there is no date range in encash request.

---

### FUNC-07: `EncashRequestOk()` Opens Multiple Dialogs Simultaneously

`encashrequest.component.ts:365-400`:
```typescript
public EncashRequestOk() {
    if (leavetypeid == "") { this.dialog.open(...) }   // dialog 1
    if (ALBalanceId == 0) { this.dialog.open(...) }    // dialog 2 — no return
    if (AEncashId == 0 || ...) { this.dialog.open(...) } // dialog 3 — no return
}
```
No `return` after each dialog open. If all three conditions are true, three dialogs stack on screen simultaneously. Also, `EncashRequestOk()` doesn't prevent the save from proceeding — it only opens dialogs but returns `void`. This appears to be a placeholder that was never connected to the Save button.

**Fix:** Add `return` after each dialog open, and ensure this method is called from `FormOutput('Save')` before proceeding.

---

### MEM-01: `isaddnew`, `issave`, `isdelete` — Set but Never Read

`encashrequest.component.ts` lines 62-66 + `rolesandrights()` (lines 402-423):
```typescript
isaddnew: boolean = false;
issave: boolean = false;
isdelete: boolean = false;

// set in rolesandrights()
this.isaddnew = false; this.issave = false; this.isdelete = false;
// or
this.isaddnew = true; this.issave = true; this.isdelete = true;
```
These three flags are set based on `OULevelSettingEncashStatus` but are never read in the template or any logic. The lock dialog opens but the form actions (AddNew / Save / Delete buttons) are not actually disabled. Dead state that misleads developers.

---

### MEM-02: `setTimeout` in `FormOutput('AddNew')`

`encashrequest.component.ts:474-476`:
```typescript
setTimeout(() => {
    this.form.get("BizTransactionTypeId")?.patchValue(this.bizid)
}, 500)
```
500ms untracked timeout. Handle not stored, not cleared in `ngOnDestroy`.

---

### MEM-03: `forkJoin` Not Protected After Component Destroy

`encashrequest.component.ts:107-168`:
```typescript
forkJoin([bizCall$, ouCall$])
    .pipe(takeUntil(this.destroy$))
    .subscribe(...)
```
This is correct — `forkJoin` is properly guarded. ✓ However, `RateandLeaveBalanceshowing()` (called in constructor) and `OnLeaveTypeChange()` use `.pipe(takeUntil(this.destroy$))` but `destroy$` is initialized after the constructor runs. Since these are in the constructor, `takeUntil` is registered before `destroy$` has its first subscriber. This is technically safe but the call should be in `ngOnInit`.

---

### API-01: `encashrequest.service.ts` Has `console.log` in `formsaveservice`

`encashrequest.service.ts:45`:
```typescript
for (let jsonvalue of JSON.ObjectFields) {
    console.log("jsonvalue=>", jsonvalue)   // logs entire JSON config on every field iteration
```
This logs every field on every save attempt — potentially 20+ log lines per save. Also at line 104: `console.log("SaveResult=>", SaveResult)`.

---

### API-02: `LoginDTODetail` in Service Never Used

`encashrequest.service.ts:28`:
```typescript
constructor(...) {
    this.LoginDTODetail = JSON.parse(sessionStorage.getItem('LoginDTO') as any);
}
```
`LoginDTODetail` is referenced in `formsaveservice` (lines 91-95) for OUId/PeriodId fallbacks, but these fallback checks are only triggered when form values are the literal strings `"OrganizationUnitId"` and `"PeriodId"` — which should never happen in normal usage. Dead sessionStorage read.

---

### API-03: No Error Handling in Any Subscribe

None of the subscribe calls have error handlers. If `biztransactionclass()`, `oulevelsettings()`, `RateandLeaveBalanceshowing()`, or `CheckAvailableLeaveBalance()` fail with network errors, the component silently shows empty/wrong data with no user feedback.

---

### CODE-01: Service Named `emirescheduledbService` in Advance Request

Looking at the advance request service, the `advancerequestDBService` is injected as `private emirescheduledbService` — clearly copied from an EMI reschedule module. Same pattern may exist in other services. Rename to match the actual class.

---

### CODE-02: Naming Convention Violations

- `encashrequestComponent` → `EncashRequestComponent`
- `encashrequestservice` → `EncashRequestService`
- `encashrequestdbservice` → `EncashRequestDbService`

---

### CODE-03: `var` Usage Throughout

`checkApplyEn()` uses `var` throughout (lines 279-321). This is a `strict: true` TypeScript codebase — `var` bypasses block scoping and creates the shadowing bugs described in FUNC-05. Use `const`/`let` everywhere.

---

### CODE-04: All Types `any`

- `loginDTO: any`, `userdatas: any`, `gridarray: any[]`, `updatedgrid: []`
- All service response parameters: `(response: any)`, `(data: any)`

---

### CODE-05: `getDateFromEpochString` Does Not Handle Null/Undefined

```typescript
public getDateFromEpochString(epochString: string): Date | null {
    const matchResult = epochString.match(/\d+/);
```
If `epochString` is `null` or `undefined` (which happens because `InsuranceStartDate` doesn't exist in the form), this throws immediately. The advance request version handles null safely; the encash version does not.

---

## Prioritized Fix List

| Priority | ID | Issue |
|----------|----|-------|
| P0 | SEC-01 | Remove sessionStorage reads (component + service) |
| P0 | FUNC-01 | Move HTTP constructor call to ngOnInit |
| P0 | FUNC-02 | Add `form.patchValue()` in `OnpicklistLoad` |
| P0 | FUNC-03 | Add parentheses to `cdr.detectChanges()` call |
| P0 | FUNC-04 | Fix `MBRId`/`AEncashId` using same source field |
| P1 | FUNC-05 | Rewrite `checkApplyEn()` with `const`/`let`, remove impossible checks |
| P1 | FUNC-06 | Fix `calculateDateDifference` to read correct date fields |
| P1 | FUNC-07 | Add `return` after each dialog in `EncashRequestOk()`, connect to Save |
| P1 | MEM-01 | Remove or wire up `isaddnew/issave/isdelete` flags |
| P1 | MEM-02 | Track setTimeout handle, clear in ngOnDestroy |
| P1 | API-01 | Remove `console.log` from service formsaveservice loop |
| P1 | API-03 | Add error handlers to all subscribe calls |
| P2 | CODE-02 | Rename classes to PascalCase |
| P2 | CODE-03 | Replace `var` with `const`/`let` |
| P2 | CODE-04 | Add TypeScript interfaces |
| P2 | CODE-05 | Add null guard to `getDateFromEpochString` |