# GbWizard Component — Re-Analysis (v2)

**Original Analysis Date:** 2026-02-24  
**Re-Analysis Date:** 2026-02-26  
**Files Analyzed:** Same as original + consumer component

---

## Summary of Changes

### ✅ Issues FIXED in Wizard Component (20 issues)

**Critical (P0):**
- C1: Added `ChangeDetectionStrategy.OnPush`
- C2: Fixed XSS risk - now uses `DomSanitizer` for `tag` field

**High Priority:**
- H1: Initial `formGroupChange` now emitted in `ngOnInit`
- H2: Added `canAdvance` input with validation guard in `next()`
- H4: Config setter no longer overwrites explicit `@Input()` bindings (uses resolved getters)
- H5: Fixed null check bug - now uses nullish coalescing operator

**Medium:**
- M2: Responsive dimensions - uses flex, `45dvh`, auto overflow
- M3: Added `trackByStepNumber` for `*ngFor`
- M4: Converted template method calls to getters
- M5: Added `catchError` in config service
- M6: Config caching with `Map` + `shareReplay`
- M7: Proper TypeScript interfaces (`WizardStepJson`, `WizardConfigJson`)
- M8: RTL support added via CSS custom properties
- M10: setTimeout now cleaned up in `ngOnDestroy`
- M11: Dialog width fixed to `min(600px, 95vw)` in consumer

**Minor/Style:**
- Mi1: Fixed `styleUrls` → `styleUrl`
- Mi2: Fixed `@Output()` naming convention
- Mi3: `overflow-y: scroll` → `auto`
- Mi4: Uses CSS custom properties for colors
- Mi6: Added `showCancel` input
- Mi7: Added full ARIA support + keyboard navigation
- Mi9: Removed dead infrastructure (`componentRefs`, etc.)

---

## ❌ Issues NOT FIXED (9 issues)

### 🔴 Critical (P0) - Consumer Component

**C3. `sessionStorage.getItem('LoginDTO')` Still Used**
- **File:** `projects/recruitment/transaction/planning/manpowerrequest/manpowerrequest.component.ts:79`
- **Current code:**
  ```typescript
  this.LoginDTO = JSON.parse(sessionStorage.getItem('LoginDTO') as any)
  ```
- **Required fix:** Inject `GbAppStateService` or auth service

**C4. Sensitive Data Still Logged to Console**
- **File:** `manpowerrequest.component.ts:80`
- **Current code:**
  ```typescript
  console.log("this.LoginDTO", this.LoginDTO)
  ```
- **Required fix:** Remove or use `GbConsoleService` (but better to remove entirely - logs sensitive user session data)

---

### 🟠 High Priority - Consumer Component

**H6. Duplicate Step Content in Template**
- **File:** `manpowerrequest.component.html`
- **Issue:** Steps are duplicated between wizard mode (lines ~27-115) and form mode (lines ~135-224)
- **Required fix:** Use single template with `<ng-template>` and toggle visibility via CSS

**H7. Duplicate Navigation State**
- **File:** `manpowerrequest.component.ts:60-71`
- **Current state:**
  ```typescript
  CurrentFormStep: number = 1;
  steps = [
      { id: 1, title: 'Basic Details' },
      { id: 2, title: 'Budget' },
      { id: 3, title: 'Requirements' },
      { id: 4, title: 'Approvers' }
  ];
  ```
- **Required fix:** Remove duplicate state; rely on wizard's `stepChange` event entirely

---

### 🟡 Medium Priority

**M1. No i18n - Hardcoded English Strings**
- **File:** `gbwizard.component.ts:50-53`
- **Current:**
  ```typescript
  @Input() nextButtonText: string = 'Next Step';
  @Input() previousButtonText: string = 'Previous';
  @Input() submitButtonText: string = 'Submit';
  @Input() cancelButtonText: string = 'Cancel';
  ```
- **Required fix:** Use Transloco keys as defaults, e.g.:
  ```typescript
  @Input() nextButtonText: string = 'wizard.buttons.next';
  ```
  And use `| transloco` pipe in template

**M12. Nested `<form>` Element**
- **File:** Need to verify in `manpowerrequest.component.html`
- **Issue:** Outer template opens `<form>` and form-mode section may have another nested form

---

### 🟢 Minor

**Mi8. Empty Test File**
- **File:** `features/gbwizard/gbwizard.component.spec.ts`
- **Status:** Still empty

---

## Remaining Work Summary

| Category | Count | Items |
|----------|-------|-------|
| 🔴 Critical (P0) | 2 | C3 (sessionStorage), C4 (console.log) |
| 🟠 High | 2 | H6 (duplicate content), H7 (duplicate state) |
| 🟡 Medium | 2 | M1 (i18n), M12 (nested form) |
| 🟢 Minor | 1 | Mi8 (empty tests) |

---

## Recommendations

### Priority 1: Fix Critical Security Issues (Consumer)

The consumer component (`ManPowerRequestComponent`) still has P0 security violations:

1. **Replace sessionStorage with proper auth injection:**
   ```typescript
   private authService = inject(GbAppStateService);
   // Use: this.authService.user() or this.authService.loginDTO()
   ```

2. **Remove console.log of sensitive data:**
   ```typescript
   // Delete line 80 entirely: console.log("this.LoginDTO", this.LoginDTO)
   ```

### Priority 2: Clean Up Consumer State

The consumer should be a thin wrapper. Remove:
- `CurrentFormStep` property (use wizard's `stepChange` event)
- `steps` array (duplicate of wizard config)
- `goToStep()` method (use wizard's API)

### Priority 3: Add i18n

Use Transloco keys for all button texts and labels.

### Priority 4: Unit Tests

Add basic tests for step navigation, formGroupChange emission, and canAdvance validation.

---

## Conclusion

The **wizard component itself is now production-ready** with 20+ issues fixed. However, the **consumer component** (`ManPowerRequestComponent`) still has 2 critical security issues (C3, C4) that must be fixed before considering this complete.

The remaining issues are primarily in the consumer rather than the shared wizard component.