# Advance Request — Deep Analysis

**Files analyzed:**
- `projects/ess/transaction/advancerequest/advancerequest.component.ts` (313 lines)
- `projects/ess/service/advancerequest.service.ts` (131 lines)
- `projects/ess/dbservice/advancerequest.db.service.ts` (59 lines)

**Date:** 2026-03-04

---

## Summary Scorecard

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

---

## P0 — Critical Issues

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

`advancerequest.component.ts:51`:
```typescript
this.loginDTO = JSON.parse(sessionStorage.getItem('LoginDTO') as any);
```
Also in `advancerequestDBService.BizTransactionClassDBService()` (line 14) — reads LoginDTO in the DB layer on every BizTransaction call. **Fix:** Use `GbAppStateService` or `GbConfigService` signal.

---

### SEC-02: `console.log` Exposes User Data

`advancerequest.component.ts:52`:
```typescript
console.log("this.loginDTO:", this.loginDTO.UserName)
```
Also in `getESSEmployee()` (line 72) and `GetEmployeeThumbnail()` (lines 104, 107). Use `GbConsoleService` or remove entirely.

---

### FUNC-01: `BiztransactionService()` Accesses `loginDTO` Before Assignment

`ngOnInit` order:
```typescript
ngOnInit(): void {
    this.BiztransactionService()           // line 49 — called first
    this.loginDTO = JSON.parse(...)        // line 51 — assigned second
```
Inside `BiztransactionService`, the success callback reads `this.loginDTO.WorkOUId` (line 128). The subscription is async so by the time the callback fires, `loginDTO` is assigned — but this is coincidental ordering that breaks if `BiztransactionService` is ever awaited synchronously or if the code order is refactored. The intent is unclear and fragile.

**Fix:** Move `loginDTO` assignment to the constructor or before any method call that depends on it.

---

### FUNC-02: `GetEmployeeThumbnail()` Calls Wrong API — Functionally Dead

`advancerequest.component.ts` lines 99-110:
```typescript
private GetEmployeeThumbnail() {
    let url = "/fws/File.svc/Get/Attachment/Meta/Data/..."  // built but NOT used
    this.service.GetmployeeSkill(this.loginDTO.UserId)      // calls user API, not file API
        .subscribe((response: any) => {
            let ans = response.responseValue[0].ViewUrl     // crashes if empty, assigned to local var
            // ans is never used
        })
}
```
Three bugs: the URL built at line 102 is never passed to the service call. The service call is the same `GetmployeeSkill` used by `getESSEmployee()`. The result `ans` is a local variable that is immediately discarded. This is dead-code HTTP that wastes a network call on every component init and every AddNew/reset.

**Fix:** Remove this method entirely, or implement it using the file attachment API endpoint.

---

### FUNC-03: `calculateDateDifference` Error Message Is Backwards

`advancerequest.component.ts:237`:
```typescript
if (date1 && date2 && date1 > date2) {
    // date1 is AdvanceAdvanceDate, date2 is AdvanceExpectedDate
    this.dialog.open(GbDialogBoxComponent, {
        message: 'From Date* should be greater than To Date*',
```
The message says "From Date should be greater than To Date" but the code triggers when From > To, which is the error case. The message should be **"Advance Date must be before Expected Date"** (or the validation is backwards).

---

## P1 — High Priority Issues

### MEM-01: Missing `ChangeDetectionStrategy.OnPush`

The component decorator does not include `changeDetection: ChangeDetectionStrategy.OnPush`. All form re-renders happen on every application CD cycle. Add OnPush.

---

### MEM-02: 4 Untracked `setTimeout` Calls

| Line | Context | Delay |
|------|---------|-------|
| 58 | `ngOnInit` — ESS employee init | 100ms |
| 74 | `getESSEmployee` — form patch | 100ms |
| 259 | `FormOutput('AddNew')` — ESS reinit | 100ms |
| 292 | `resetForm()` — ESS reinit | 100ms |

None of the handles are stored. If component is destroyed while a timer is pending, the callback runs on a destroyed component instance. **Fix:** Store handles in a field and clear in `ngOnDestroy`.

---

### API-01: `BiztransactionService()` Called 3 Times

The same HTTP call to `/ads/BizTransactionType.svc/Rights/SelectList` is fired:
1. On `ngOnInit` (line 49)
2. On `FormOutput('AddNew')` (line 258)
3. On `resetForm()` (line 291)

The result (BizTransactionTypeId and Name) doesn't change between calls. Cache the result after first load:
```typescript
private bizTransactionResult: any = null;

private BiztransactionService(): void {
  if (this.bizTransactionResult) {
    this.applyBizTransactionResult(this.bizTransactionResult);
    return;
  }
  this.service.BizTransactionClassService(this.BizTransactionClassId)
    .pipe(takeUntil(this.destroy$))
    .subscribe(result => {
      this.bizTransactionResult = result;
      this.applyBizTransactionResult(result);
    });
}
```

---

### API-02: `advancerequestservice.formsaveservice` — `LoginDTODetail` Is Never Assigned

`advancerequest.service.ts` declares `LoginDTODetail: any` at line 21 but the constructor (line 24) never assigns it. The `formsaveservice` method reads `this.LoginDTODetail.WorkPeriodId` / `WorkOUId` (lines 87–93). These fallback checks are broken and will throw `Cannot read properties of null` if triggered.

**Fix:** Inject `GbAppStateService` and read the signal, or remove the dead fallback logic.

---

### API-03: URL Typo in `advancerequestDBService.getBizType`

`advancerequest.db.service.ts:51`:
```typescript
let url = '/ads/BizTransactioanType.svc/?BizTransactionTypeId=' + id
//                      ^^^^^ typo: "Transactioan" instead of "Transaction"
```
This URL will return 404. The BizType detail load (`bizSettingLoad`) always fails silently — `isAuto`/`isManual` flags are never set from the API.

---

### CODE-01: Naming Convention Violations

- Class name `advancerequestComponent` — Angular convention requires `PascalCase`: `AdvanceRequestComponent`
- Class name `advancerequestservice` → `AdvanceRequestService`
- Class name `advancerequestDBService` → `AdvanceRequestDbService`

---

### CODE-02: `isAuto`/`isManual` Are Mutually Exclusive — Should Be One Field

The component has two boolean flags that are always set as opposites:
```typescript
this.isAuto = true; this.isManual = false;
// or
this.isAuto = false; this.isManual = true;
```
This is error-prone (they can be out of sync). Use a single field:
```typescript
generationType: 'auto' | 'manual' | 'none' = 'none';
```

---

### CODE-03: `DrillDownDetails` Injected but Never Used

`advancerequestComponent` constructor injects `DrillDownDetails: IDrillDownDetails` (line 43) but never reads it anywhere. Remove the unused injection.

---

### CODE-04: `ZeroValidation` Parameter Name Misleads

```typescript
ZeroValidation(event: any): void {
    let value = event;           // "event" is actually a numeric value, not an Event object
    value = Number(value);
```
Rename to `validateAmount(value: number)`.

---

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

- `loginDTO: any`, `UserDetail: any`, `bizTransactionClassId: any`, `GenerationType: any`
- Service return types: `any`
- DB service parameters: `id: any`

Define interfaces for LoginDTO (already exists: `ILoginDTO`), BizTransactionType, and employee data.

---

### CODE-06: Constructor Injection in Service Layer

`advancerequestservice` constructor (line 24) uses constructor injection for `FormActiondbservice`, `HttpClient`, and `advancerequestDBService`. **Fix:** Migrate to `inject()`.

---

### CODE-07: Semicolon Bug in `ngOnDestroy`

```typescript
this.destroy$.complete();; // double semicolon (cosmetic but indicates copy-paste)
```

---

## Service Layer Issues

### `advancerequestservice.formsaveservice` — Not Using Transloco Consistently

Validation alert messages are passed through `this.translate.translate()` for field labels, but the error messages themselves ("Should not be Empty", "Please Provide Valid Percentage") are hardcoded English. If the app runs in another language, the field label is translated but the surrounding text remains English.

### `advancerequestDBService` Architecture

The DB service reads `sessionStorage` directly (line 14) instead of receiving `OUId` and `PeriodId` as parameters. This ties the DB service to the browser storage mechanism and makes unit testing impossible. Pass IDs as parameters.

---

## Prioritized Fix List

| Priority | ID | Issue |
|----------|----|-------|
| P0 | SEC-01 | Remove sessionStorage LoginDTO reads (component + DB service) |
| P0 | SEC-02 | Remove console.log of user data |
| P0 | FUNC-01 | Ensure loginDTO assigned before first use |
| P0 | FUNC-02 | Remove dead `GetEmployeeThumbnail()` HTTP call |
| P0 | FUNC-03 | Fix backwards date validation message |
| P1 | API-03 | Fix URL typo `BizTransactioanType` → `BizTransactionType` |
| P1 | API-02 | Fix null `LoginDTODetail` crash in formsaveservice |
| P1 | MEM-01 | Add `ChangeDetectionStrategy.OnPush` |
| P1 | MEM-02 | Track setTimeout handles, clear in ngOnDestroy |
| P1 | API-01 | Cache BiztransactionService result (called 3×) |
| P2 | CODE-01 | Rename classes to PascalCase |
| P2 | CODE-02 | Replace isAuto/isManual with single union type |
| P2 | CODE-03 | Remove unused DrillDownDetails injection |
| P2 | CODE-04 | Rename ZeroValidation parameter |
| P2 | CODE-05 | Add interfaces, remove `any` types |
| P2 | CODE-06 | Migrate service constructors to inject() |