# GbFilterComponent — In-Depth Analysis **File:** `features/gbfilter/gbfilter.component.ts` (1,746 lines) **Date:** 2026-02-24 **Analyst:** Claude Code ## Overview Core filter component used across 20+ modules for report/screen filtering. The architecture is sound at a high level (Component → Service → DbService) and the model interfaces in `ifilter.model.ts` are well-defined, but there are critical bugs, significant performance problems, several memory leaks, and the component needs decomposition. **Files analysed:** - `features/gbfilter/gbfilter.component.ts` (1,746 lines) - `features/gbfilter/gbfilter.component.html` (564 lines) - `features/gbfilter/gbfilter.component.scss` (1,575 lines) - `features/gbfilter/ifilter.model.ts` (328 lines) - `features/gbfilter/service/gbfilter.service.ts` (34 lines) - `features/gbfilter/dbservice/gbfilter.dbservice.ts` (43 lines) --- ## 1. Critical Bugs ### 1.1 `parseDate()` is a no-op — both branches do the same thing `gbfilter.component.ts:863–870` ```typescript parseDate(dateString: string): Date { let ConvertedDate = new Date(new Date(dateString).toUTCString()).getTime() + 19800000 if (/^\d{4}-\d{2}-\d{2}$/.test(dateString)) { return new Date(ConvertedDate); // ← same } else { return new Date(ConvertedDate); // ← same } } ``` The IST offset `19800000` is hardcoded — breaks for any non-IST deployment. Used only in `onDateSelected()` for from > to comparison; the offset cancels out so the comparison still works, but the implementation is wrong. --- ### 1.2 Wrong date format string `'dd-mm-yyyy'` (minutes, not months) `gbfilter.component.ts:764, 769, 808, 810, 814, 816` `datePipe.transform(..., 'dd-mm-yyyy')` — lowercase `mm` is **minutes**, not months. Should be `dd-MM-yyyy`. Affects period presets `"3"` (YTD), `"10"` (Previous Year YTD), and `"11"` (Previous Year), causing completely wrong dates for all financial-year-relative calculations. --- ### 1.3 `radiobtnclicked()` case `"2"` (MTD) logic is wrong `gbfilter.component.ts:754–761` ```typescript let dayDifference = currentDate.getDate() - lastDayOfMonth.getDate(); // always ≤ 0 let adjustedLastDay = new Date(..., currentDate.getMonth() + 1, dayDifference); // day=0 → last day of month; day=-N → goes back N days further ``` MTD should be start-of-month to today. This code produces a wrong end date. --- ### 1.4 `WorkPeriodFromDate` parsed differently in two places In `ngOnChanges` the robust `parseApiDate()` helper is used for computing `Fromdate`/`Todate`. But in `radiobtnclicked()` cases `"3"`, `"10"`, `"11"`, the raw `WorkPeriodFromDate` is manually stripped using `.replace('/Date(', '').replace(')/', '')` and `JSON.parse()`d. This throws if `WorkPeriodFromDate` is an ISO string (not `.NET /Date()/ ` format). Two completely different parsing strategies for the same field. --- ### 1.5 `ApplyFilter()` appends `"000"` to already-millisecond timestamp `gbfilter.component.ts:1401–1407` ```typescript let filterdate = filterdata.FieldValue.getTime() // already milliseconds // ... filterdata.CriteriaAttributeValue = datePipe.transform(`${filterdata.FieldValue}000`, ...) // FieldValue is in seconds after line 1406; appending "000" puts the date in year ~52000 ``` --- ### 1.6 `#scrollContainer` ViewChild declared twice in the template `gbfilter.component.html:72` and `gbfilter.component.html:302` Angular resolves `@ViewChild('scrollContainer')` to the **last** match. The outer tab content div is ignored; scroll-to-section is relative to the inner `.filter-modal-wrapper` div, making offset calculations wrong. --- ### 1.7 `AddOnPicklistdata.Picklistdatavalues` grows on every `ReportViewId` change `gbfilter.component.ts:461–469` — no `= []` reset before pushing. Switching views accumulates duplicate entries unboundedly. --- ### 1.8 `openJobSubmission()` contains hardcoded test data `gbfilter.component.ts:1674–1693` ```typescript menuid: 'MENU_001', username: 'John Doe', userid: 'USER_123' // test stub left in production ``` Should use actual menu and user data from `MenuDetail` and `LoginDto`. --- ### 1.9 "Show all Filters" toggle and mobile search have no bindings `gbfilter.component.html:199–202` — `` with no `[(ngModel)]` or `(change)` handler. Does nothing. `gbfilter.component.html:267–270` — mobile `` search field with no `[(ngModel)]` or `(input)` binding. Does nothing. --- ### 1.10 Direct DOM manipulation in `CreatingFilterArray()` `gbfilter.component.ts:1195–1198` ```typescript const radio = document.getElementById('Others') as HTMLInputElement; if (radio) { radio.checked = true; } ``` Bypasses Angular's rendering cycle. Will silently fail if the element hasn't rendered yet (e.g., `*ngIf` is false). --- ### 1.11 `IIsMultiple` logic is inverted `gbfilter.component.ts:1150` ```typescript Multiple: data.IIsMultiple == 0 ? true : false ``` Value `0` enables multi-select, which is counter-intuitive. The field name `IsMultiple` implies `1` means true. --- ### 1.12 `radiobtnclicked()` case `"7"` sets both dates to the same value `gbfilter.component.ts:786–791` — "Previous Year DTD" sets both `Fromdate` and `Todate` to the same date one year ago. Likely intended to be start-of-that-year's-week to that date. --- ## 2. Memory Leaks ### 2.1 `valueChanges` subscription in `ngOnInit` not cleaned up `gbfilter.component.ts:310–312` ```typescript this.filterform.get('AttributesCriteriaList')?.valueChanges.subscribe(val => { this.filteredFormData = val; }); // Missing: .pipe(takeUntil(this.destroy$)) ``` Worse: `CreatingFilterArray()` recreates `filterform` entirely — the old subscription keeps firing on a detached form, causing stale updates. --- ### 2.2 `GetReportFormatService` subscription in `ngOnChanges` not cleaned up `gbfilter.component.ts:341–349` — if `MenuDetail` changes while a request is in-flight, the prior callback still fires. Missing `takeUntil(this.destroy$)` (used correctly elsewhere in the component). --- ### 2.3 `Renderer2` injected but never used `gbfilter.component.ts:297` — dead injection keeping a reference alive unnecessarily. --- ## 3. Performance Issues ### 3.1 No `ChangeDetectionStrategy.OnPush` `gbfilter.component.ts:86–97` — missing entirely. The component manages a `FormArray` with potentially 30+ controls, a `MatTree`, and a section nav list. Every app-wide event triggers full change detection on all of this. --- ### 3.2 `getAvailableOperators()` called from template without memoization `gbfilter.component.ts:1209–1229` — parses a comma-delimited string and filters the operators list on **every change detection cycle** for **every visible filter row**. With 15 rows this is called ~900 times/second while scrolling. Should be computed once per row and cached. --- ### 3.3 `buildTreeFromForm()` deep-clones via slow JSON roundtrip `gbfilter.component.ts:699` ```typescript this.originalTreeData = JSON.parse(JSON.stringify(treeData)); ``` Use `structuredClone()` or avoid storing a copy by searching directly against the form data. --- ### 3.4 `performSearch()` has no debounce `gbfilter.component.ts:177` — fires on every `(input)` event, rebuilds the tree, and schedules a `setTimeout` for expand on every keystroke. A 300ms debounce is needed. --- ### 3.5 `CreatingFilterArray()` rebuilds entire `FormGroup` from scratch on every change `gbfilter.component.ts:1075–1079` — called on every `ngOnChanges` for `MenuDetail`. Creates 30+ `FormGroup`s, triggers tree rebuild and section name load. Patch existing controls instead of destroying and recreating. --- ### 3.6 `new DatePipe("en-US")` instantiated inside loops and methods `gbfilter.component.ts:712, 1085, 1108, 1308, 1401, 1697` — six separate instantiations. Should be injected once via `inject(DatePipe)`. --- ### 3.7 Dead O(n²) methods that would be catastrophic if re-enabled `shouldShowSectionHeader()` (line 615) and `getSectionName()` (line 641) each do `findIndex` over all controls for every item — O(n²) per change detection cycle. Both are currently unused but commented templates suggest they may return. --- ## 4. Architecture & Standards Violations | Issue | Location | Severity | |---|---|---| | No `ChangeDetectionStrategy.OnPush` | ts:86 | High | | Constructor injection mixed with `inject()` | ts:294 vs ts:297 | Medium | | 6 out of 8 `@Input()` typed `any` | ts:99–107 | High | | All major class properties typed `any` | ts:115–177 | High | | `sessionStorage.getItem('LoginDTO')` | ts:350 | P0 Security | | 6× `console.log/warn/error` | ts:181, 430, 1035, 1104, 1128, 1167 | Medium | | NGXS `store.dispatch()` — should be signals | ts:1492 | Medium | | `MatDialog` fixed `width: '600px'` — 8 occurrences | ts:854, 1374, etc. | Low | | No Transloco — all display strings hardcoded English | html throughout | Medium | | No `public-api.ts` export barrel | — | Low | | `ConfugurationPicklist` typo (6× in code) | ts:246, 1174, etc. | Low | | 1,746-line component — too large to maintain | ts:98–1746 | High | | Dead methods: `shouldShowSectionHeader`, `getSectionName`, `getGroupedItems` | ts:580, 615, 641 | Low | | `ConfugurationPicklist` object literal duplicated 6× | ts:246, 1174, 1274, 1327, 1556, 1602 | Medium | | `FilterType` typed as `string` not union type | ifilter.model.ts:248 | Medium | | `PeriodFromArray` / `PeriodToArray` typed `any[]` instead of `string[]` | ts:133–134 | Low | --- ## 5. Security Issues | Issue | Location | |---|---| | `sessionStorage.getItem('LoginDTO')` — exposes full login payload | ts:350 | | `LoginDto.UserId`, `WorkOUId` embedded inline in picklist URL params | ts:355, 380 | | `RecordsPerPage` is a raw `` with no numeric validation | html:224 | --- ## 6. Functional Improvements ### 6.1 Period presets don't reliably sync form controls `radiobtnclicked()` updates `this.Fromdate`/`this.Todate` class properties. These are `[(ngModel)]`-bound to date inputs which also have `formControlName="FieldValue"`. Mixing `ngModel` and `formControlName` on the same element is an Angular anti-pattern — the form control value may lag the displayed value, so `ApplyFilter()` could send stale dates. --- ### 6.2 Saved config uses magic number IDs for date fields `gbfilter.component.ts:1310–1313` ```typescript if (ConfigurationValue.CriteriaAttributeId == -2147483643) { // From Date — magic ID if (ConfigurationValue.CriteriaAttributeId == -2147483642) { // To Date — magic ID ``` These should be named constants, e.g. `FROM_DATE_CRITERIA_ID = -2147483643`. --- ### 6.3 `ApplyFilter()` corrupts `radioSelected` after applying After applying, the map at line 1478–1483 permanently remaps `radioSelected` — "PreviousWeek" → "0", "WTD" → "1". If the user re-opens the filter without refreshing, the dropdown shows the wrong selection. --- ### 6.4 Validation error messages are not localizable `gbfilter.component.ts:1388`: `'Please Fill ' + filterdata.Picklistobject.Label` — string concatenation, not Transloco keys. Will not support Arabic or any other language. --- ### 6.5 `NUMERICENTRY` allows invalid input via paste `gbfilter.component.ts:1242–1245` — `validateInput()` strips non-numeric characters on `input` event, but the field is `type="text"`. Paste events can insert invalid characters before the handler fires. Use `type="number"` or reactive form validators. --- ### 6.6 `Showallrecords` and `RecordsPerPage` not mutually exclusive in UI When `Showallrecords = true`, `RecordsPerPage` becomes `-1`. But the UI shows both active simultaneously — checking "Show all Data" should visually disable/clear the records-per-page input. --- ### 6.7 Section navigation uses spaces in element IDs `gbfilter.component.ts:508`: `document.getElementById('section-Filter Addons')` — spaces in IDs is invalid HTML. Should use kebab-case or encode the section name. --- ### 6.8 `FilterType` should be a union type In `IFilterArray` and throughout the component `FilterType: string` is used. Should be: ```typescript type FilterType = 'PICKLIST' | 'LIST' | 'DATE' | 'NUMERICENTRY' | 'TEXTENTRY'; ``` This would catch typos at compile time and enable exhaustive switch checks. --- ### 6.9 `LIST` filter sends index not value `gbfilter.component.html:358–363` — the `` has 12 operators with no "None" option and no "StartWith"/"EndWith", unlike the main `OperatorsList` (14 entries). These should share a single source of truth. --- ## 7. Recommended Decomposition The 1,746-line component should be split into focused components: ``` GbFilterComponent (orchestration + open/close only, ~150 lines) ├── GbFilterDateRangeComponent (date inputs + period presets + radio/select) ├── GbFilterFieldsComponent (section nav + tree + search) │ └── GbFilterFieldRowComponent (single row: label + operator + input control) ├── GbFilterConfigComponent (save/load/delete config tab) └── GbFilterAddonComponent (addon picklist + operator table) ``` Benefits: - Each piece can have its own `ChangeDetectionStrategy.OnPush` - Config and Addon sections load lazily (not rendered until switched to) - Easier to unit test each concern independently - Reduces re-render surface when only one section changes --- ## 8. Priority Fix Order | Priority | Fix | Impact | |---|---|---| | **P0** | `'dd-mm-yyyy'` → `'dd-MM-yyyy'` in `radiobtnclicked()` | Broken dates in prod for YTD/prev-year presets | | **P0** | Fix MTD (`"2"`) calculation in `radiobtnclicked()` | Wrong date range sent to backend | | **P0** | Clear `AddOnPicklistdata.Picklistdatavalues` before repopulating | Duplicate addon entries | | **P0** | Fix `#scrollContainer` duplicate ViewChild | Scroll-to-section broken | | **P1** | Add `takeUntil(this.destroy$)` to `valueChanges` subscription | Memory leak + stale callbacks | | **P1** | Add `takeUntil(this.destroy$)` to `GetReportFormatService` | In-flight request callbacks after navigation | | **P1** | Add `ChangeDetectionStrategy.OnPush` | Performance at scale | | **P1** | Memoize `getAvailableOperators()` result per row | CD performance | | **P1** | Remove `sessionStorage.getItem('LoginDTO')` — use auth service | Security | | **P1** | Fix `openJobSubmission()` — replace hardcoded stub data | Functional correctness | | **P1** | Fix `ApplyFilter()` date `"000"` timestamp bug | Wrong epoch sent to backend | | **P2** | Debounce `performSearch()` | UI responsiveness | | **P2** | Inject `DatePipe` once instead of `new DatePipe()` inline | Minor perf | | **P2** | Extract `ConfugurationPicklist` builder to a private method | Maintainability | | **P2** | Bind "Show all Filters" toggle and mobile search input | Dead UI controls | | **P2** | Fix `radioSelected` corruption after `ApplyFilter()` | UX bug | | **P2** | Add `FilterType` union type | Type safety | | **P3** | Decompose component into 4–5 focused components | Long-term maintainability | | **P3** | Replace NGXS `store.dispatch()` with signals | Per project standards | | **P3** | Add Transloco i18n for all display strings | Arabic/multi-language support | | **P3** | Dialog widths → `min(Xpx, 95vw)` | Responsive | | **P3** | Remove dead methods and commented-out blocks | Code hygiene |