feat(migration): initialize Angular 19 scaffold and port auth flow#67
feat(migration): initialize Angular 19 scaffold and port auth flow#67Anurag9699 wants to merge 1 commit into
Conversation
📝 WalkthroughWalkthroughThis PR introduces a complete Angular 19 scaffold for the Helpline 104 project. It establishes the foundational application structure, core services for authentication and state management, HTTP interceptor, layout and feature components, and a Zard UI component library (button, dialog, input, toast) with custom event manager plugins and shared utilities. The project is configured for strict TypeScript, Tailwind CSS styling with dark mode support, and environment-based configuration. ChangesAngular 19 Project Scaffold
🎯 4 (Complex) | ⏱️ ~60 minutes Possibly Related Issues
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
|
There was a problem hiding this comment.
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
🟠 Major comments (26)
helpline104-v19/src/app/shared/core/provider/event-manager-plugins/zard-debounce-event-manager-plugin.ts-5-27 (1)
5-27:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDebounce parsing is too positional and can silently strip modifiers.
Line 6 accepts many
.debounceplacements, but Line 18 assumesevent.debounce.delay. Inputs likekeydown.enter.debounce.150or chained modifiers can be misparsed (wrong delay and/or dropped suffix behavior).Proposed fix
export class ZardDebounceEventManagerPlugin extends EventManagerPlugin { override supports(eventName: string): boolean { - return /\.debounce(?:\.|$)/.test(eventName); + return /(^|\.)debounce(?:\.\d+)?(?:\.|$)/.test(eventName); } override addEventListener( element: HTMLElement, eventName: string, handler: (event: Event) => void, options?: ListenerOptions, // eslint-disable-next-line ): Function { - // Expected format: "event.debounce.delay" (e.g., "input.debounce.150") - // If delay is omitted or invalid, defaults to 300ms - const [event, , delay] = eventName.split('.'); - const parsedDelay = Number.parseInt(delay); - const resolvedDelay = Number.isNaN(parsedDelay) ? 300 : parsedDelay; + const parts = eventName.split('.'); + const debounceIndex = parts.indexOf('debounce'); + if (debounceIndex <= 0) { + return this.manager.addEventListener(element, eventName, handler, options); + } + + const maybeDelay = parts[debounceIndex + 1]; + const hasDelay = /^\d+$/.test(maybeDelay ?? ''); + const resolvedDelay = hasDelay ? Number(maybeDelay) : 300; + + const forwardedEventName = [ + ...parts.slice(0, debounceIndex), + ...parts.slice(debounceIndex + (hasDelay ? 2 : 1)), + ].join('.'); let timeoutId!: ReturnType<typeof setTimeout>; const listener = (event: Event) => { clearTimeout(timeoutId); timeoutId = setTimeout(() => handler(event), resolvedDelay); }; - const unsubscribe = this.manager.addEventListener(element, event, listener, options); + const unsubscribe = this.manager.addEventListener(element, forwardedEventName, listener, options); return () => { clearTimeout(timeoutId); unsubscribe(); }; } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@helpline104-v19/src/app/shared/core/provider/event-manager-plugins/zard-debounce-event-manager-plugin.ts` around lines 5 - 27, The debounce parsing assumes a fixed positional format and can drop modifiers; update addEventListener (and keep supports as-is) to locate the 'debounce' token anywhere in eventName by splitting on '.' and finding the index of the token equal to 'debounce', then read the next token (if any) as the delay to parse into parsedDelay/resolvedDelay (default 300ms), and reconstruct the actual event string by removing the 'debounce' token and its numeric token so that modifiers like 'enter' remain; then wire timeoutId, listener, and unsubscribe to call this.manager.addEventListener with the reconstructed event name and same options/handler logic.helpline104-v19/src/app/app.routes.ts-8-22 (1)
8-22:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAuthenticated routes are unguarded — any unauthenticated user can access
/role-selectionand/dashboarddirectly.The PR introduces a full auth flow (
AuthService, login with PBKDF2+AES, session storage) but never wires it to the router. Both therole-selectionroute and the entire authenticated shell (path: '') are reachable without credentials.The
canActivateguard determines whether a user can access a route and is most commonly used for authentication and authorization. Angular's modern approach uses function-based guards with no class boilerplate — just a simple function usinginject()when dependencies are needed.The most idiomatic fix is:
- Add a functional
authGuardincore/guards/auth.guard.tsthat delegates toAuthService.- Apply
canActivate: [authGuard]to therole-selectionroute.- Apply
canActivateChild: [authGuard]on thepath: ''shell route so all current and future child routes are protected automatically.🔒 Proposed fix
// core/guards/auth.guard.ts (new file) +import { inject } from '@angular/core'; +import { CanActivateFn, Router } from '@angular/router'; +import { AuthService } from '../services/auth.service'; + +export const authGuard: CanActivateFn = () => { + const auth = inject(AuthService); + const router = inject(Router); + return auth.isAuthenticated() || router.createUrlTree(['/login']); +};// app.routes.ts -import { Routes } from '@angular/router'; +import { Routes } from '@angular/router'; +import { authGuard } from './core/guards/auth.guard'; export const routes: Routes = [ { path: 'login', loadComponent: () => import('./features/auth/login/login').then(m => m.LoginComponent) }, { path: 'role-selection', + canActivate: [authGuard], loadComponent: () => import('./features/dashboard/role-selection/role-selection').then(m => m.RoleSelectionComponent) }, { path: '', + canActivateChild: [authGuard], loadComponent: () => import('./core/layout/main-layout/main-layout').then(m => m.MainLayoutComponent), children: [🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@helpline104-v19/src/app/app.routes.ts` around lines 8 - 22, Add a function-based auth guard (create core/guards/auth.guard.ts exporting authGuard) that injects AuthService and returns its isAuthenticated check (or redirects/returns UrlTree via Router) and then wire it into routes: add canActivate: [authGuard] to the role-selection route and add canActivateChild: [authGuard] to the shell route (the route with path: '' that loads MainLayoutComponent) so AuthService is consulted for both the standalone role-selection and all children like the Dashboard component.helpline104-v19/src/environments/environment.development.ts-3-3 (1)
3-3:⚠️ Potential issue | 🟠 Major | ⚡ Quick winEmpty
encKeywill break AES encryption at runtime or produce trivially weak ciphertext.
session-storage.service.tsderivesSECRET_KEYfromenvironment.encKey. An empty string as an AES key either throws in the crypto library or silently produces deterministic ciphertext that is trivially reversible — making all session data effectively unencrypted in the dev build. Use a non-empty placeholder key (it does not need to match production) so the encryption path is exercised correctly during development and testing.🛠️ Proposed fix
- encKey: '', + encKey: 'dev-only-placeholder-key-32bytes!',🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@helpline104-v19/src/environments/environment.development.ts` at line 3, environment.encKey is empty which breaks AES usage; set a non-empty development placeholder for environment.encKey so session-storage.service.ts can derive a valid SECRET_KEY and exercise encryption in dev. Update the value in environment.development.ts to a hard-coded non-secret placeholder string (e.g., "dev-secret-placeholder") so AES initialization paths in session-storage.service.ts/SECRET_KEY run without errors; ensure this placeholder is clearly identified as dev-only and not used in production.helpline104-v19/src/environments/environment.development.ts-6-6 (1)
6-6:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
telephoneServerpoints to a live UAT host over plain HTTP.Unlike every other URL in this file (which are all
localhost),telephoneServeris set to an external server (uatcz.piramalswasthya.org) over unencrypted HTTP. Any developer running the app locally will send traffic — potentially containing PII or health data — in plaintext to the UAT environment. Switch tohttps://and consider whether alocalhoststub or mock is more appropriate for the development environment.🛠️ Proposed fix
- telephoneServer: 'http://uatcz.piramalswasthya.org/', + telephoneServer: 'https://uatcz.piramalswasthya.org/',🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@helpline104-v19/src/environments/environment.development.ts` at line 6, The telephoneServer value is pointing to an external UAT host over plain HTTP; update the telephoneServer constant to use HTTPS (e.g., "https://uatcz.piramalswasthya.org/") or, better for local development, replace it with a localhost stub/mock URL (e.g., "http://localhost:PORT/") or a configurable env reference so dev builds do not send plaintext or real PII to the UAT environment; modify the telephoneServer entry accordingly and ensure any code reading telephoneServer (e.g., HTTP client initialization) continues to work with the new value.helpline104-v19/src/environments/environment.ts-3-3 (1)
3-3:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
encKey: ''— empty encryption key weakens AES protection to a known constant
SessionStorageServicederives itsSECRET_KEYfromenvironment.encKey. An empty key means either the fallback is a hardcoded constant or AES receives an empty passphrase — both are trivially brute-forceable. Ensure this is populated from a CI secret or build-time injection before any deployment, and add an explicit runtime guard inSessionStorageServicethat throws when the key is empty.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@helpline104-v19/src/environments/environment.ts` at line 3, The environment.encKey is empty which makes AES-derived SECRET_KEY in SessionStorageService insecure; populate encKey via CI/build-time secret injection and add a runtime guard in SessionStorageService that checks the derived/loaded SECRET_KEY (or environment.encKey) and throws an error if it is missing/empty; update any bootstrap/startup code to fail fast on missing encKey so deployments break early and document that encKey must come from secrets (refer to encKey in environment.ts and SECRET_KEY/SessionStorageService for the exact locations to change).helpline104-v19/package.json-34-34 (1)
34-34:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
xlsx@0.18.5has two unpatched high-severity CVEs on npm
- CVE-2023-30533 (CVSS 7.8) — Prototype Pollution; CVE-2024-22363 (CVSS 7.5) — ReDoS
- A non-vulnerable version cannot be found via npm, as the repository hosted on GitHub and the npm package
xlsxare no longer maintained. CVE-2024-22363 was resolved in 0.20.2, available via the SheetJS CDN.- The
xlsxpackage maintenance is considered inactive and hasn't seen any new versions released to npm in the past 12 months.Consider migrating to ExcelJS (actively maintained, available on npm) or pinning via the SheetJS CDN tarball if you must keep xlsx.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@helpline104-v19/package.json` at line 34, The package.json currently depends on the vulnerable "xlsx" package; replace it with a maintained alternative or pin a safe SheetJS tarball: either remove "xlsx" and add "exceljs" (update any imports/usages that reference "xlsx" to use ExcelJS APIs), or change the "xlsx" entry to a CDN-pinned tarball/version known to include the fix (e.g., the SheetJS 0.20.2 tarball) and update build/install scripts accordingly; ensure all code paths that call into the "xlsx" module are updated to the new package's API (search for require/import "xlsx" and adapt those call sites).helpline104-v19/src/environments/environment.ts-6-6 (1)
6-6:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
telephoneServeruses plain HTTP against an external domainAll other base URLs target
localhost(acceptable for dev), buttelephoneServer: 'http://uatcz.piramalswasthya.org/'references a live UAT host over unencrypted HTTP. Any auth token or session data sent to that endpoint will be transmitted in plaintext. Change tohttps://or use a localhost stub for the default dev environment.🛡️ Proposed fix
- telephoneServer: 'http://uatcz.piramalswasthya.org/', + telephoneServer: 'https://uatcz.piramalswasthya.org/',🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@helpline104-v19/src/environments/environment.ts` at line 6, The environment.ts file sets telephoneServer to an external URL over plain HTTP; update the telephoneServer value in the environment.ts configuration (symbol: telephoneServer) to use a secure HTTPS endpoint (e.g., change 'http://uatcz.piramalswasthya.org/' to 'https://uatcz.piramalswasthya.org/') or replace it with a localhost stub suitable for local development so credentials/tokens are not sent over plaintext.helpline104-v19/src/app/core/interceptors/auth.interceptor.ts-17-22 (1)
17-22:⚠️ Potential issue | 🟠 Major | ⚡ Quick winForcing
Content-Type: application/jsonon every request breaks non-JSON uploadsFile uploads using
multipart/form-datarequire the browser to set the boundary-encodedContent-Typeautomatically. Overriding it unconditionally strips theboundaryparameter, causing the server to reject the body. Skip the header for requests that already have aContent-Typeor carry aFormData/Blobbody.🐛 Proposed fix
- let authReq = req.clone({ - setHeaders: { - 'Content-Type': 'application/json', - 'Authorization': authService.getToken() || '' - } - }); + const headers: Record<string, string> = { + 'Authorization': authService.getToken() || '' + }; + // Only set Content-Type for non-multipart requests + if (!req.body || !(req.body instanceof FormData)) { + headers['Content-Type'] = 'application/json'; + } + let authReq = req.clone({ setHeaders: headers });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@helpline104-v19/src/app/core/interceptors/auth.interceptor.ts` around lines 17 - 22, The interceptor currently forces 'Content-Type: application/json' on every request (see authReq and req.clone), which breaks multipart uploads; modify the logic to only set 'Content-Type' when the outgoing request has no existing 'Content-Type' header and the body is not a FormData/Blob/ArrayBuffer/ReadableStream; preserve setting 'Authorization' using authService.getToken() as before, and if a Content-Type exists on req.headers or req.body is instance of FormData/Blob, clone without adding Content-Type to avoid stripping boundary parameters.helpline104-v19/src/styles.scss-93-93 (1)
93-93:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
--destructive-foregroundis referenced in@themebut never defined in:rootor.darkLine 93 maps
--color-destructive-foreground: var(--destructive-foreground), but--destructive-foregroundhas no declared value in either:rootor.dark. Any component usingbg-destructive-foregroundortext-destructive-foregroundwill render with an empty (transparent/inherited) color.🐛 Proposed fix — add missing variable to both `:root` and `.dark`
:root { ... --destructive: oklch(0.577 0.245 27.325); + --destructive-foreground: oklch(0.985 0 0); --border: oklch(0.922 0 0);.dark { ... --destructive: oklch(0.704 0.191 22.216); + --destructive-foreground: oklch(0.985 0 0); --border: oklch(1 0 0 / 10%);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@helpline104-v19/src/styles.scss` at line 93, The theme references --destructive-foreground but that variable is never defined, causing bg-destructive-foreground/text-destructive-foreground to render empty; fix by adding a concrete value for --destructive-foreground in both :root and .dark (matching your destructive color system) so --color-destructive-foreground: var(--destructive-foreground) resolves correctly—update the declarations in the :root block and the .dark block to include --destructive-foreground (and adjust the dark-mode variant value accordingly).helpline104-v19/src/app/core/services/auth.service.ts-18-25 (1)
18-25:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
getToken()/removeToken()bypassSessionStorageService, likely returning ciphertext
SessionStorageServiceapplies AES encryption. If'authToken'was stored through it,sessionStorage.getItem('authToken')returns the encrypted blob — not the plaintext token. The interceptor would then send ciphertext as theAuthorizationheader, causing every authenticated request to fail with a 401.Either store the auth token unencrypted directly via
sessionStorage.setItem(and document that deliberately), or route throughSessionStorageService:🐛 Proposed fix (route through SessionStorageService)
getToken(): string | null { - return sessionStorage.getItem('authToken'); + return this.sessionService.getItem('authToken'); } removeToken() { - sessionStorage.removeItem('authToken'); - sessionStorage.removeItem('apiman_key'); + this.sessionService.removeItem('authToken'); + this.sessionService.removeItem('apiman_key'); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@helpline104-v19/src/app/core/services/auth.service.ts` around lines 18 - 25, getToken() and removeToken() are directly calling sessionStorage and thus return/store the AES-encrypted blob instead of the plaintext token; update both methods to use the project's SessionStorageService (e.g., call SessionStorageService.getItem('authToken') and SessionStorageService.removeItem('authToken') / removeItem('apiman_key')) so the token is decrypted before the HTTP interceptor uses it, or if you intentionally want an unencrypted token, consistently store/retrieve it via sessionStorage.setItem/getItem and document that choice — change getToken/removeToken to one consistent approach and keep references to getToken, removeToken and the SessionStorageService to locate the change.helpline104-v19/package.json-9-9 (1)
9-9:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
"test": "ng test"will not run VitestThe PR description and AI summary confirm Vitest is the configured test runner (referenced in
.vscode/launch.json/tasks.json).ng testinvokes the Angular CLI test builder (Karma/Jest), not Vitest, sonpm testin CI silently runs the wrong runner (or errors out if no Karma config exists).🐛 Proposed fix
- "test": "ng test" + "test": "vitest run", + "test:watch": "vitest"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@helpline104-v19/package.json` at line 9, The package.json "test" npm script currently runs the Angular CLI ("ng test") but this repo uses Vitest; update the "test" script in package.json (the "test" npm script entry) to invoke Vitest (e.g., "vitest" or "vitest run" as appropriate) so npm test and CI run the configured Vitest runner; ensure any related devDependency (vitest) is installed and update CI/.vscode tasks if they rely on the old "ng test" command.helpline104-v19/src/app/core/layout/main-layout/main-layout.ts-111-116 (1)
111-116:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAvoid redirecting authenticated users on refresh
At Line 114, auth gating is based only on volatile
dataService.Userdata; a refresh can clear it and send valid sessions to/login. Gate redirect on token presence (and ideally rehydrate user state) before navigating.Suggested fix
ngOnInit() { - // If no user data is present in DataService, they probably refreshed. - // In a real app, we'd fetch it from session/localStorage or redirect to login. - if (!this.dataService.Userdata) { - this.router.navigate(['/login']); - } + const token = this.authService.getToken(); + if (!this.dataService.Userdata && !token) { + this.router.navigate(['/login'], { replaceUrl: true }); + } + // If token exists but in-memory user is empty, rehydrate user state here. }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@helpline104-v19/src/app/core/layout/main-layout/main-layout.ts` around lines 111 - 116, The current ngOnInit() redirects to /login solely on absence of dataService.Userdata which can be cleared on page refresh; instead check for a persisted auth token or AuthService.isAuthenticated() before redirecting and attempt to rehydrate Userdata (e.g., call a DataService.rehydrateFromToken(token) or AuthService.fetchUser()) if a token exists—only call router.navigate(['/login']) if no token/session is present or rehydration fails. Locate ngOnInit, dataService.Userdata and router.navigate in main-layout.ts and add the token/session check and rehydrate call before performing the redirect.helpline104-v19/src/app/core/services/session-storage.service.ts-9-9 (1)
9-9:⚠️ Potential issue | 🟠 Major | ⚡ Quick winFail fast when encryption key is missing
Line 9 silently falls back to
'default-key', which creates a predictable key in production misconfigurations. This should hard-fail instead of weakening confidentiality.Suggested fix
export class SessionStorageService { - private readonly SECRET_KEY = environment.encKey || 'default-key'; + private readonly SECRET_KEY: string; + + constructor() { + if (!environment.encKey) { + throw new Error('Missing environment.encKey'); + } + this.SECRET_KEY = environment.encKey; + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@helpline104-v19/src/app/core/services/session-storage.service.ts` at line 9, Replace the silent fallback for SECRET_KEY so the service fails fast when the encryption key is absent: remove the "|| 'default-key'" from the private readonly SECRET_KEY initialization and add a runtime check (e.g., in the SessionStorageService constructor or an init method) that throws a clear Error if environment.encKey is missing or empty; reference the SECRET_KEY field and SessionStorageService so reviewers can locate the change and ensure all callers still rely on a validated SECRET_KEY rather than a hardcoded default.helpline104-v19/src/app/core/layout/main-layout/main-layout.ts-48-74 (1)
48-74:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRestore keyboard focus visibility and explicit labels for icon buttons
Lines 48-74 remove focus outlines and depend on
title. Addaria-labelplus visiblefocus-visiblestyles so keyboard users can operate header actions reliably.Suggested fix
-<button class="hover:text-primary transition-colors focus:outline-none" title="Emergency Contacts"> +<button + class="hover:text-primary transition-colors focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-primary rounded" + title="Emergency Contacts" + aria-label="Emergency Contacts" +> ... -<button class="hover:text-primary transition-colors flex items-center focus:outline-none" title="Profile"> +<button + class="hover:text-primary transition-colors flex items-center focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-primary rounded" + title="Profile" + aria-label="Profile" +> ... -<button class="hover:text-primary transition-colors focus:outline-none" title="Help"> +<button + class="hover:text-primary transition-colors focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-primary rounded" + title="Help" + aria-label="Help" +> ... -<button (click)="logout()" class="hover:text-red-500 transition-colors focus:outline-none" title="Logout"> +<button + (click)="logout()" + class="hover:text-red-500 transition-colors focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-red-500 rounded" + title="Logout" + aria-label="Logout" +>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@helpline104-v19/src/app/core/layout/main-layout/main-layout.ts` around lines 48 - 74, The header icon buttons removed keyboard focus visibility and rely only on title attributes; restore accessible keyboard focus and explicit labels by adding aria-label attributes to each button (e.g., the Emergency Contacts button, the Profile button that opens the dropdown, the Help button, and the Logout button which calls logout()), and replace the blanket "focus:outline-none" usage with visible focus-visible styles such as "focus:outline-none focus-visible:ring-2 focus-visible:ring-primary focus-visible:ring-offset-2" (or equivalent) so keyboard users see focus; keep the title attributes and the existing ng-icon usage and ensure the profile dropdown button still exposes aria-expanded/aria-haspopup if necessary.helpline104-v19/src/app/core/services/session-storage.service.ts-39-56 (1)
39-56:⚠️ Potential issue | 🟠 Major | ⚡ Quick winHarden cookie handling (encoding + SameSite/Secure)
Lines 39-56 write raw cookie values and omit
SameSite/Secure, which increases parsing bugs and security exposure. Encode values and set safer defaults.Suggested fix
- setCookie(name: string, value: string, days: number) { - let expires = ""; - if (days) { - const date = new Date(); - date.setTime(date.getTime() + (days * 24 * 60 * 60 * 1000)); - expires = "; expires=" + date.toUTCString(); - } - document.cookie = name + "=" + (value || "") + expires + "; path=/"; - } + setCookie(name: string, value: string, days: number): void { + let expires = ''; + if (days) { + const date = new Date(); + date.setTime(date.getTime() + days * 24 * 60 * 60 * 1000); + expires = `; expires=${date.toUTCString()}`; + } + const secure = window.location.protocol === 'https:' ? '; Secure' : ''; + document.cookie = + `${encodeURIComponent(name)}=${encodeURIComponent(value ?? '')}` + + `${expires}; path=/; SameSite=Lax${secure}`; + } - getCookie(name: string) { - const nameEQ = name + "="; + getCookie(name: string): string | null { + const nameEQ = `${encodeURIComponent(name)}=`; const ca = document.cookie.split(';'); for (let i = 0; i < ca.length; i++) { let c = ca[i]; while (c.charAt(0) === ' ') c = c.substring(1, c.length); - if (c.indexOf(nameEQ) === 0) return c.substring(nameEQ.length, c.length); + if (c.indexOf(nameEQ) === 0) return decodeURIComponent(c.substring(nameEQ.length, c.length)); } return null; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@helpline104-v19/src/app/core/services/session-storage.service.ts` around lines 39 - 56, The setCookie/getCookie helpers currently write and read raw values and omit security attributes; update setCookie to URL-encode the value (use encodeURIComponent), always append ; SameSite=Strict and ; Secure (retain existing Path and Expires logic), and ensure value is safely included even when empty; update getCookie to decode the retrieved value with decodeURIComponent before returning and trim cookie entries robustly in getCookie; modify the functions named setCookie and getCookie accordingly so consumers receive encoded/decoded values and cookies are set with SameSite=Strict and Secure by default.helpline104-v19/src/app/shared/core/directives/string-template-outlet/string-template-outlet.directive.ts-96-98 (1)
96-98:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
#isFirstChangetoggling forces repeated template view recreation.On Lines 96-98,
#isFirstChangegets set back totruefor template outlets, so template updates can keep recreating the embedded view instead of updating context.Proposed fix
- if (!this.#isFirstChange && isTemplateRef(stringTemplateOutlet)) { - this.#isFirstChange = true; - } - if (!isTemplateRef(stringTemplateOutlet)) { this.context['$implicit'] = stringTemplateOutlet as T; } @@ `#updateTrackingState`( stringTemplateOutlet: TemplateRef<void> | T, stringTemplateOutletContext: ZardStringTemplateOutletContext | undefined, ): void { const isTemplate = isTemplateRef(stringTemplateOutlet); - if (this.#isFirstChange && !isTemplate) { + if (this.#isFirstChange) { this.#isFirstChange = false; }Also applies to: 75-83, 104-111
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@helpline104-v19/src/app/shared/core/directives/string-template-outlet/string-template-outlet.directive.ts` around lines 96 - 98, The directive is incorrectly resetting the private flag `#isFirstChange` to true when the outlet is a TemplateRef (isTemplateRef) which forces repeated embedded view recreation; instead, stop toggling `#isFirstChange` back to true — treat it as a one-time "first change" marker (only clear it once on initial change) and remove the branches that set `#isFirstChange` = true (the occurrences around isTemplateRef at the blocks referencing stringTemplateOutlet and the other similar blocks at the 75-83 and 104-111 locations); update the logic in the string-template-outlet directive so that `#isFirstChange` only transitions from true→false on first update and is never set back to true, ensuring embedded views are updated via context changes rather than being recreated.helpline104-v19/src/app/shared/components/dialog/dialog.component.ts-35-52 (1)
35-52:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
zTitletype promisesTemplateRef, but only string rendering is implemented.
ZardDialogOptions.zTitleallowsTemplateRef<T>(Line 51), but Line 77 always interpolates it as text. This is a broken API contract.Proposed fix (contract-safe quick option)
- zTitle?: string | TemplateRef<T>; + zTitle?: string;Also applies to: 77-77
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@helpline104-v19/src/app/shared/components/dialog/dialog.component.ts` around lines 35 - 52, ZardDialogOptions.zTitle allows TemplateRef<T> but the dialog template always interpolates it as text; update the dialog rendering to honor TemplateRef values: in the dialog component template (where zTitle is currently interpolated) add an *ngIf to detect if zTitle is a string vs a TemplateRef and render strings with interpolation but render TemplateRef using <ng-container *ngTemplateOutlet="zTitle as TemplateRef"> or similar (or use ngTemplateOutlet with a type guard method on the component), ensuring the component import of TemplateRef and the ZardDialogOptions.zTitle usage both support TemplateRef<T> correctly.helpline104-v19/src/app/shared/components/button/button.component.ts-125-133 (1)
125-133:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDetect host element type from the host tag, not its parent.
On Lines 128-130, checking
parentElement.tagNamemisclassifies native<button z-button>and<a z-button>, which then applies wrong ARIA/role/tabindex behavior.Proposed fix
- protected readonly isNotInsideOfButtonOrLink = computed(() => { - // Evaluated once; assumes component parent doesn't change after mount - const zardButtonElement = this.elementRef.nativeElement; - if (zardButtonElement.parentElement) { - const { tagName } = zardButtonElement.parentElement; - return tagName !== 'BUTTON' && tagName !== 'A'; - } - return true; - }); + protected readonly isNotInsideOfButtonOrLink = computed(() => { + const hostTag = this.elementRef.nativeElement.tagName; + return hostTag !== 'BUTTON' && hostTag !== 'A'; + });Also applies to: 47-47
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@helpline104-v19/src/app/shared/components/button/button.component.ts` around lines 125 - 133, The computed property isNotInsideOfButtonOrLink is incorrectly checking the host's parent tagName so native usages like <button z-button> or <a z-button> get misclassified; change the check to inspect the host element itself via this.elementRef.nativeElement.tagName (normalize to upper-case) and return false when the host tagName is 'BUTTON' or 'A'; make the same fix for the other occurrence referenced (the similar check near line 47) so ARIA/role/tabindex logic uses the host tag type rather than its parent.helpline104-v19/src/app/core/services/login.service.ts-61-64 (1)
61-64:⚠️ Potential issue | 🟠 Major | ⚡ Quick winClear
authTokenduring logout to avoid stale Authorization usage.On Lines 62-63,
authTokenis left behind insessionStorage, which can keep downstream auth state inconsistent after logout.Proposed fix
userLogout() { sessionStorage.removeItem('privilege_flag'); sessionStorage.removeItem('session_id'); + sessionStorage.removeItem('authToken'); return this.http.post<any>(this.logoutUserUrl, {}).pipe(🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@helpline104-v19/src/app/core/services/login.service.ts` around lines 61 - 64, The userLogout method leaves authToken in sessionStorage causing stale Authorization usage; update userLogout (the function calling this.logoutUserUrl) to also remove the 'authToken' key from sessionStorage (alongside 'privilege_flag' and 'session_id') before making the post and returning the HTTP observable so the app cannot reuse a cleared session's token.helpline104-v19/src/app/shared/components/button/button.component.ts-40-48 (1)
40-48:⚠️ Potential issue | 🟠 Major | ⚡ Quick winCustom
role="button"host needs keyboard activation handlers.When rendered as
<z-button>, Enter/Space currently don’t trigger click behavior, which blocks keyboard-only interaction.Proposed fix
host: { '[class]': 'classes()', @@ '[attr.role]': 'isNotInsideOfButtonOrLink() ? "button" : null', - '[attr.tabindex]': 'isNotInsideOfButtonOrLink() ? "0" : null', + '[attr.tabindex]': 'isNotInsideOfButtonOrLink() ? (zDisabled() ? "-1" : "0") : null', + '(keydown.enter)': 'onKeyboardActivate($event)', + '(keydown.space)': 'onKeyboardActivate($event)', }, @@ export class ZardButtonComponent implements OnDestroy { @@ + protected onKeyboardActivate(event: KeyboardEvent): void { + if (!this.isNotInsideOfButtonOrLink() || this.zDisabled()) return; + event.preventDefault(); + this.elementRef.nativeElement.click(); + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@helpline104-v19/src/app/shared/components/button/button.component.ts` around lines 40 - 48, The host currently sets role="button" but lacks keyboard activation; add host bindings for keydown and keyup (e.g., '(keydown)': 'onHostKeyDown($event)', '(keyup)': 'onHostKeyUp($event)') and implement onHostKeyDown/onHostKeyUp methods on the component (alongside existing classes(), iconOnly(), isNotInsideOfButtonOrLink(), zDisabled()) to emulate native button behavior: if isNotInsideOfButtonOrLink() && !zDisabled(), treat Enter as activation (call native click or trigger the component's click handler on keydown or keyup as appropriate), handle Space by preventing default on keydown to avoid page scroll and activating on keyup, and ignore repeated/long-press key events; ensure checks for disabled state and children being actual button/link are applied before activating.helpline104-v19/src/app/shared/components/input/input.directive.ts-129-139 (1)
129-139:⚠️ Potential issue | 🟠 Major | ⚡ Quick winNumeric parsing should not depend on previous model type.
On Lines 132-139, conversion to number runs only when current model is
number | null, so numeric inputs can emit strings for common initial states.Proposed fix
private readNativeValue(element: ZardInputElement | null): ZardInputValue { @@ if (this.isNumericInput(element)) { - const currentValue = this.value(); - - if (typeof currentValue === 'number' || currentValue === null) { - if (element.value === '') { - return null; - } - - const numericValue = element.valueAsNumber; - return Number.isNaN(numericValue) ? null : numericValue; - } + if (element.value === '') { + return null; + } + const numericValue = element.valueAsNumber; + return Number.isNaN(numericValue) ? null : numericValue; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@helpline104-v19/src/app/shared/components/input/input.directive.ts` around lines 129 - 139, The numeric input parsing currently only runs if the existing model value (this.value()) is number|null, which lets numeric fields emit strings initially; update the isNumericInput branch in the input directive so it always parses the DOM value regardless of the previous model type: when isNumericInput(element) is true, treat empty string as null, otherwise read element.valueAsNumber and return null for NaN or the numeric value; adjust the logic in the method that contains isNumericInput, value(), and element handling so numeric inputs consistently emit numbers.helpline104-v19/src/app/features/auth/login/login.ts-178-189 (1)
178-189:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUse filtered
104privileges for gating and role extraction.On Lines 182-189, you filter to
privileges104but continue withres.previlegeObj[0]. This can admit non-104users and can throw ifroles[0]is missing.Proposed fix
- if (res && res.previlegeObj && res.previlegeObj.length > 0) { + if (res && Array.isArray(res.previlegeObj) && res.previlegeObj.length > 0) { this.dataService.Userdata = res; - // Filter privileges for '104' service as per legacy logic const privileges104 = res.previlegeObj.filter((p: any) => p.serviceName === '104'); + if (privileges104.length === 0) { + this.loginResult.set("User doesn't have privilege to access 104"); + return; + } + this.dataService.userPriveliges = privileges104; this.dataService.uid = res.userID; this.dataService.agentID = res.agentID; this.dataService.uname = this.userID.trim(); - this.sessionService.setItem('privilege_flag', res.previlegeObj[0].roles[0].RoleName); + const roleName = privileges104[0]?.roles?.[0]?.RoleName; + if (!roleName) { + this.loginResult.set("User doesn't have privilege to access 104"); + return; + } + this.sessionService.setItem('privilege_flag', roleName); sessionStorage.setItem('authToken', res.key);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@helpline104-v19/src/app/features/auth/login/login.ts` around lines 178 - 189, The code filters privileges into privileges104 but still reads res.previlegeObj[0].roles[0].RoleName which can be the wrong service or undefined; update the logic in the login handler to use the filtered privileges104 (e.g., check privileges104.length > 0), pick the appropriate entry (e.g., privileges104[0]) and safely access roles (ensure roles && roles.length > 0) before setting sessionService.setItem('privilege_flag', ...), and fall back to a safe default or skip setting the flag if no 104 privilege/role exists; adjust any related assignments that rely on res.previlegeObj[0] to reference the filtered privileges104 instead.helpline104-v19/src/app/core/services/login.service.ts-24-34 (1)
24-34:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMove
withCredentialsto HttpClient options instead of request payload.
withCredentials: trueon line 28 is being sent as JSON data, which won't enable cookie credentials on the request. It must be passed in the HttpClient options (third parameter).This same issue exists in both
helpline104-v19/src/app/core/services/login.service.tsandsrc/app/services/loginService/login.service.ts.Proposed fix
authenticateUser(uname: string, pwd: string, doLogout: boolean, captchaToken?: string) { const body: any = { userName: uname, password: pwd, - withCredentials: true, doLogout: doLogout, }; if (captchaToken) body.captchaToken = captchaToken; - return this.http.post<any>(`${this.baseUrl}user/userAuthenticate`, body).pipe( + return this.http.post<any>(`${this.baseUrl}user/userAuthenticate`, body, { withCredentials: true }).pipe(🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@helpline104-v19/src/app/core/services/login.service.ts` around lines 24 - 34, The authenticateUser function is sending withCredentials in the JSON body instead of enabling credentials on the HTTP request; remove withCredentials from the request payload in authenticateUser (and the duplicate in the other login service) and pass it as the HttpClient post options (third parameter) like { withCredentials: true } when calling this.http.post<any>(...). Ensure you still include captchaToken when present and doLogout/userName/password remain in the body.helpline104-v19/src/app/features/dashboard/role-selection/role-selection.ts-163-165 (1)
163-165:⚠️ Potential issue | 🟠 Major | ⚡ Quick winClear the
DataServicerole state on logout too.This component populates
current_role,current_roleID,current_feature,current_service,screens, andagentID, but logout only clears browser storage. If the user logs in again without a full reload, those in-memory values can bleed into the next session until something overwrites them.Suggested fix
logout() { + this.dataService.resetSessionState?.(); sessionStorage.clear(); this.router.navigate(['/login']); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@helpline104-v19/src/app/features/dashboard/role-selection/role-selection.ts` around lines 163 - 165, The logout() method currently only clears sessionStorage and navigates to /login; you must also clear the in-memory role-related state on the shared DataService to avoid leakage across logins. Update logout() to reset DataService's role-related fields (current_role, current_roleID, current_feature, current_service, screens, agentID) — either by calling a dedicated reset method on DataService (e.g., resetRoleState()) if one exists or by setting those properties to null/empty values via this.dataService.<property> = null/[] as appropriate — so both sessionStorage and the DataService state are cleared on logout.helpline104-v19/src/app/features/dashboard/role-selection/role-selection.ts-101-121 (1)
101-121:⚠️ Potential issue | 🟠 Major | ⚡ Quick winBlock navigation when no feature mapping was resolved.
getSelectedFeature()can return''here, butselectRole()still emitsroleSelectedand routes to/dashboardwith partially updatedDataService. That lets a malformed role reuse the previous selection'scurrent_feature/current_role/screensstate. Bail out before mutating global state unless a valid feature+role pair was resolved.Suggested fix
- this.getSelectedFeature(role); + const selectedFeature = this.getSelectedFeature(role); + if (!selectedFeature || !this.dataService.current_role) { + this.dataService.current_feature = ''; + this.dataService.current_role = ''; + this.dataService.screens = []; + return; + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@helpline104-v19/src/app/features/dashboard/role-selection/role-selection.ts` around lines 101 - 121, selectRole() calls getSelectedFeature() which can return an empty value, but the code proceeds to mutate DataService (current_roleName/current_roleID/current_service/agentID) and emit roleSelected and navigate to /dashboard anyway; update selectRole() to check the value returned by getSelectedFeature() and if it's falsy/empty, return early (do not set dataService.current_feature/current_role/screens, do not emit roleSelected, and do not call router.navigate), optionally surface a validation/error message; reference getSelectedFeature() and the selectRole() block that sets dataService.current_roleName, current_roleID, current_service, agentID, roleSelected.next(...) and router.navigate(['/dashboard']) to locate where to add the guard.helpline104-v19/src/app/shared/components/dialog/dialog.service.ts-94-102 (1)
94-102:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPass the caller's
ViewContainerRefintoTemplatePortal.Angular CDK's
TemplatePortalrequires a non-nullViewContainerRefto stamp the template into the correct context. Passingnull as unknown as ViewContainerRefat line 98 bypasses this requirement and will cause runtime failures when template-backed dialogs are created. The same config field (config.zViewContainerRef) is already correctly used for component portals at line 107, so use it here as well. SincezViewContainerRefis optional in the config, add a validation check to fail fast with a clear error message instead of silent failures.Suggested fix
if (componentOrTemplateRef instanceof TemplateRef) { + if (!config.zViewContainerRef) { + throw new Error('Template dialogs require zViewContainerRef'); + } + dialogContainer.attachTemplatePortal( new TemplatePortal<T>( componentOrTemplateRef, - null as unknown as ViewContainerRef, + config.zViewContainerRef, { dialogRef, } as T, ), );🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@helpline104-v19/src/app/shared/components/dialog/dialog.service.ts` around lines 94 - 102, The TemplatePortal is being constructed with a null ViewContainerRef which causes runtime errors; update the branch that handles componentOrTemplateRef instanceof TemplateRef to pass config.zViewContainerRef into new TemplatePortal (use the same zViewContainerRef used for component portals) and add a guard that validates config.zViewContainerRef exists before calling dialogContainer.attachTemplatePortal—if missing, throw an explicit Error (or use dialogRef.close/error) with a clear message referencing TemplatePortal and zViewContainerRef so callers fail fast.
🟡 Minor comments (6)
helpline104-v19/src/index.html-5-5 (1)
5-5:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winReplace scaffolded
<title>with a user-facing value.
Helpline104V19is the Angular CLI default derived from the project name and appears in the browser tab and bookmarks for end users.✏️ Proposed fix
- <title>Helpline104V19</title> + <title>Helpline 104</title>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@helpline104-v19/src/index.html` at line 5, The <title> tag currently contains the scaffolded value "Helpline104V19"; replace that text in the <title> element in index.html with a concise, user-facing string (for example "Helpline 104 — [short description]") that will appear in the browser tab and bookmarks; ensure the new title is human-readable, concise, and reflects the app purpose (or wire it to a dynamic value via Angular's Title service if you need runtime updates).helpline104-v19/src/app/shared/utils/number.ts-9-11 (1)
9-11:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
min === maxcauses division-by-zero inconvertValueToPercentage, returningNaN.When
minandmaxare equal the denominator(max - min)is0, yieldingNaN, which breaks any progress-bar or percentage display that consumes this value.🛡️ Proposed guard
function convertValueToPercentage(value: number, min: number, max: number): number { + if (max === min) return 0; return ((value - min) / (max - min)) * 100; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@helpline104-v19/src/app/shared/utils/number.ts` around lines 9 - 11, convertValueToPercentage currently divides by (max - min) which yields NaN when min === max; update convertValueToPercentage to guard against that by handling the equal-bound case explicitly (e.g., if max === min return 0 when value <= min else 100) and ensure the computed percentage is clamped to the 0–100 range so callers (progress bars) never receive NaN or out-of-bound values.helpline104-v19/.vscode/launch.json-12-18 (1)
12-18:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
ng testdebug URL targets Karma, but the project uses Vitest.
http://localhost:9876/debug.htmlis Karma's debug page. Sincetsconfig.spec.jsondeclaresvitest/globalsas the test type provider, Vitest is the test runner — it does not expose a/debug.htmlendpoint on port 9876. This launch configuration will fail to attach a debugger to tests.For Vitest's browser mode or UI mode, the debug URL differs (typically
http://localhost:51204/for@vitest/browseror the Vitest UI port). Update the URL to match your Vitest setup, or remove this config until Vitest debugging is configured.🛠️ Example fix for Vitest UI mode
{ "name": "ng test", "type": "chrome", "request": "launch", "preLaunchTask": "npm: test", - "url": "http://localhost:9876/debug.html" + "url": "http://localhost:51204/" }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@helpline104-v19/.vscode/launch.json` around lines 12 - 18, The launch configuration named "ng test" points to Karma's debug URL (http://localhost:9876/debug.html) which won't work because this project uses Vitest; update the "url" value in .vscode/launch.json for the "ng test" configuration to your Vitest debug/UI endpoint (e.g. the Vitest UI or `@vitest/browser` port) or remove/disable the "ng test" launch entry until you have a proper Vitest debugging URL configured so the Chrome debugger can attach correctly.helpline104-v19/src/app/shared/utils/number.ts-5-7 (1)
5-7:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
step = 0causes a division-by-zero, returningNaN.If
stepis0,(value - min) / stepproducesInfinityorNaN, and the function silently returns a bad value that propagates into any slider or step-based UI component consuming it.🛡️ Proposed guard
function roundToStep(value: number, min: number, step: number): number { + if (step === 0) return value; return Math.round((value - min) / step) * step + min; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@helpline104-v19/src/app/shared/utils/number.ts` around lines 5 - 7, roundToStep can divide by zero when step === 0 causing NaN; add an early guard in the roundToStep function to handle step === 0 (or non-positive) and return a sensible fallback (e.g., return the input value or min) or throw a clear error instead of performing the division; update roundToStep's parameter validation so it checks step === 0 (and optionally step <= 0) before computing Math.round((value - min) / step) to avoid the division-by-zero.helpline104-v19/src/app/shared/components/input/input.variants.ts-32-34 (1)
32-34:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winReorder Tailwind v4 variant chain for
file:responsive stylesAt Lines 32-34,
file:max-md:...violates Tailwind v4 variant ordering. In v4, responsive variants must be applied before pseudo-elements. Usemax-md:file:...instead.Suggested fix
- { zType: 'default', zSize: 'default', class: 'h-9 py-1 file:h-7 file:text-sm file:max-md:py-0' }, - { zType: 'default', zSize: 'sm', class: 'h-8 py-1 file:h-6 file:text-sm file:max-md:py-1.5' }, - { zType: 'default', zSize: 'lg', class: 'h-10 py-1 file:h-7 file:text-sm file:max-md:py-2.5' }, + { zType: 'default', zSize: 'default', class: 'h-9 py-1 file:h-7 file:text-sm max-md:file:py-0' }, + { zType: 'default', zSize: 'sm', class: 'h-8 py-1 file:h-6 file:text-sm max-md:file:py-1.5' }, + { zType: 'default', zSize: 'lg', class: 'h-10 py-1 file:h-7 file:text-sm max-md:file:py-2.5' },🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@helpline104-v19/src/app/shared/components/input/input.variants.ts` around lines 32 - 34, The Tailwind variant ordering in the class strings for the input variants is incorrect: any occurrence of the chain "file:max-md:..." should be reordered to "max-md:file:..." so responsive variants come before the pseudo-element; update the three objects in the input variants array (the entries with zType: 'default' and zSize 'default' | 'sm' | 'lg') by replacing "file:max-md:" with "max-md:file:" in their class values.helpline104-v19/src/app/shared/components/dialog/dialog.component.ts-74-83 (1)
74-83:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRender
zDescriptionindependently ofzTitle.On Lines 79-81, description is nested inside the title block, so a description-only dialog won’t show the description.
Proposed fix
`@if` (config.zTitle || config.zDescription) { <header class="flex flex-col space-y-1.5 text-center sm:text-left"> `@if` (config.zTitle) { <h4 data-testid="z-title" class="text-lg leading-none font-semibold tracking-tight">{{ config.zTitle }}</h4> - - `@if` (config.zDescription) { - <p data-testid="z-description" class="text-muted-foreground text-sm">{{ config.zDescription }}</p> - } + } + `@if` (config.zDescription) { + <p data-testid="z-description" class="text-muted-foreground text-sm">{{ config.zDescription }}</p> } </header> }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@helpline104-v19/src/app/shared/components/dialog/dialog.component.ts` around lines 74 - 83, The description (config.zDescription) is currently rendered only when config.zTitle exists because the paragraph is nested inside the title if-block; update the template in dialog.component.ts so the <p data-testid="z-description"> block is moved out of the `@if` (config.zTitle) { ... } block and rendered whenever config.zDescription is truthy (i.e., separate `@if` (config.zDescription) { <p data-testid="z-description">... } ), keeping the header and title rendering (data-testid="z-title") unchanged.
|
Hi @Anurag9699 @drtechie has this issue already been assigned?. |
Overview
This PR introduces the initial foundation for migrating the legacy Helpline104-UI (Angular 4) application to a modern Angular 19 standalone architecture.
The focus of this PR is:
Establishing a clean, scalable base structure
Migrating core infrastructure
Rebuilding the authentication and role selection flow
This aligns with Phase 1 (Core Setup) and Phase 2 (Authentication Flow) of the migration plan.
Migration Approach
Instead of directly modifying the legacy codebase, this PR follows a side-by-side migration strategy:
A new Angular 19 application is created inside:
helpline104-v19/
The legacy system remains untouched for:
reference
validation
logic comparison
This approach ensures:
Zero risk to the existing system
Controlled and incremental migration
Easier debugging and validation of business logic
Key Implementations
Initialized Angular 19 project using standalone components (no NgModule)
Created structured folders:
core/ — services and interceptors
features/ — modular feature flows
shared/ — reusable UI components
Replaced deprecated @angular/http with HttpClient
Implemented a functional interceptor for:
token injection
API handling
Designed with scalability and maintainability in mind
Refactored legacy dataService into:
core/services/data.service.ts
Preserved existing functionality while improving code structure and readability
Reimplemented:
SessionStorageService
AES-based encryption compatible with backend
Rebuilt login flow using:
Zard UI
Tailwind CSS
Preserved:
PBKDF2 + AES encryption logic
Backend compatibility
Refactored legacy MultiRoleScreenComponent into:
MainLayoutComponent (layout shell)
RoleSelectionComponent (separated UI logic)
Improved:
UI clarity
separation of concerns
maintainability
Verification
Application builds successfully (ng build passes)
Encryption logic verified against legacy implementation
Role mapping validated using DataService state
No breaking changes introduced to backend integration
Why This PR Matters
This PR establishes the base for the complete migration by:
Introducing a scalable and maintainable architecture
Aligning the project with modern Angular practices
Enabling a safe and structured transition from the legacy system
Next Steps
Port shared directives and pipes
Begin dashboard module migration
Gradually migrate remaining components
Summary by CodeRabbit