Skip to content

feat(migration): initialize Angular 19 scaffold and port auth flow#67

Open
Anurag9699 wants to merge 1 commit into
PSMRI:mainfrom
Anurag9699:migration/angular-19-phase-1
Open

feat(migration): initialize Angular 19 scaffold and port auth flow#67
Anurag9699 wants to merge 1 commit into
PSMRI:mainfrom
Anurag9699:migration/angular-19-phase-1

Conversation

@Anurag9699

@Anurag9699 Anurag9699 commented May 7, 2026

Copy link
Copy Markdown

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

  1. Core Architecture Setup
    Initialized Angular 19 project using standalone components (no NgModule)
    Created structured folders:
    core/ — services and interceptors
    features/ — modular feature flows
    shared/ — reusable UI components
  2. HTTP Layer Modernization
    Replaced deprecated @angular/http with HttpClient
    Implemented a functional interceptor for:
    token injection
    API handling
    Designed with scalability and maintainability in mind
  3. State and Storage Refactor
    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
  4. Authentication Flow (End-to-End)
    Rebuilt login flow using:
    Zard UI
    Tailwind CSS
    Preserved:
    PBKDF2 + AES encryption logic
    Backend compatibility
  5. Role Selection Refactor
    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

  • New Features
    • Added comprehensive authentication system with secure login and role-based access control.
    • Implemented dashboard interface with main layout including header, navigation, and footer.
    • Added role selection functionality for user privilege management.
    • Introduced reusable UI components for buttons, dialogs, input fields, and notifications.
    • Configured application styling with Tailwind CSS theming and animations.

@coderabbitai

coderabbitai Bot commented May 7, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

This 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.

Changes

Angular 19 Project Scaffold

Layer / File(s) Summary
Build & TypeScript Configuration
tsconfig.json, tsconfig.app.json, tsconfig.spec.json, angular.json, .editorconfig, .gitignore, .prettierrc, .postcssrc.json, components.json
Strict TypeScript mode with Angular 19 application config, app/spec compilation scopes, editor formatting rules, git ignore patterns, Prettier formatting, PostCSS Tailwind integration, and component schema with path aliases.
Dependencies & Package Management
package.json
Angular 19 core/router/forms, Zard UI, utility libraries (clsx, crypto-js, date-fns, lucide-angular, ngx-sonner, socket.io), and dev tools (TypeScript, Vitest, Prettier, PostCSS, Tailwind).
VS Code Workspace
.vscode/extensions.json, .vscode/launch.json, .vscode/mcp.json, .vscode/tasks.json
Extension recommendations, Chrome debug configs for ng serve/ng test, Angular CLI MCP server, npm task runners with TypeScript matchers.
Application Entry Point
src/index.html, src/main.ts, src/app/app.ts, src/app/app.html
HTML entry point with app-root mount, Angular bootstrap via bootstrapApplication, root App component with RouterOutlet, default title signal.
Routing & Application Config
src/app/app.routes.ts, src/app/app.config.ts
Routes for login, role-selection, lazy-loaded layout with nested dashboard, catch-all redirect; appConfig wires HTTP interceptor, router, and event plugins.
Styling & Theme System
src/styles.scss
Tailwind CSS with light/dark theme CSS variables, @theme mapping, base layer styles, scrollbar/input normalization.
Environment Configuration
src/environments/environment.ts, src/environments/environment.development.ts
Development/production API endpoints, encryption/CAPTCHA keys, telephoneServer URL, feature flags.
Core Services
src/app/core/services/auth.service.ts, src/app/core/services/login.service.ts, src/app/core/services/session-storage.service.ts, src/app/core/services/data.service.ts, src/app/core/services/loader.service.ts
Token management, login API with error normalization, AES-encrypted session storage with cookies, user/privilege state, loader visibility signals.
HTTP Interceptor & Layout
src/app/core/interceptors/auth.interceptor.ts, src/app/core/layout/main-layout/main-layout.ts
Request loader display, Authorization/apikey injection, 401/403 error handling with redirect; sticky header, user info display, logout handler.
Feature Components
src/app/features/auth/login/login.ts, src/app/features/dashboard/role-selection/role-selection.ts, src/app/features/dashboard/dashboard/dashboard.ts
Client-side PBKDF2/AES password encryption, login form; role selection with RO/HAO privilege filtering; dashboard stub.
Button Component
src/app/shared/components/button/button.component.ts, src/app/shared/components/button/button.variants.ts, src/app/shared/components/button/index.ts
Zard button with type/size/shape variants, MutationObserver icon-only detection, loading/disabled states, host bindings, and barrel exports.
Dialog Component Library
src/app/shared/components/dialog/...
CDK Overlay-based dialogs, portal attachment, template/component content, animation lifecycle, click handlers, mask closability, variants, and imports barrel.
Input Component
src/app/shared/components/input/input.directive.ts, src/app/shared/components/input/input.variants.ts, src/app/shared/components/input/index.ts
ControlValueAccessor input directive, numeric handling, variant styling, data-slot binding, form integration, and barrel exports.
Toast Component
src/app/shared/components/toast/toast.component.ts, src/app/shared/components/toast/toast.variants.ts, src/app/shared/components/toast/index.ts
NgxSonnerToaster wrapper, configurable position/theme/variant/options, and barrel exports.
Shared Directives & Utilities
src/app/shared/core/directives/id.directive.ts, src/app/shared/core/directives/string-template-outlet/..., src/app/shared/utils/merge-classes.ts, src/app/shared/utils/number.ts, src/app/shared/utils/index.ts, src/app/shared/core/index.ts
ID generation, dynamic template/value rendering, class merging via clsx/tailwind-merge, numeric bounds/rounding/percentage conversion, truncation detection, and core index re-exports.
Event Manager Plugins
src/app/shared/core/provider/providezard.ts, src/app/shared/core/provider/event-manager-plugins/zard-event-manager-plugin.ts, src/app/shared/core/provider/event-manager-plugins/zard-debounce-event-manager-plugin.ts
Custom event modifiers (.prevent, .stop, .debounce), plugin registration via provideZard().
Documentation
README.md
Project name, Angular CLI version, development/build/test/e2e instructions, CLI resources.

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly Related Issues

Poem

🐰 A burrow of code, neat and grand,
With Angular's threads, so well-fanned,
Zard buttons dance, dialogs gleam,
Services flowing in a perfect stream,
From login to role, the path is clear—
Welcome, dear Helpline, we're building here! 🌟

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'feat(migration): initialize Angular 19 scaffold and port auth flow' directly summarizes the main changes: Angular 19 project initialization and authentication flow migration. It accurately reflects the primary purpose of this large-scale codebase migration.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 win

Debounce parsing is too positional and can silently strip modifiers.

Line 6 accepts many .debounce placements, but Line 18 assumes event.debounce.delay. Inputs like keydown.enter.debounce.150 or 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 win

Authenticated routes are unguarded — any unauthenticated user can access /role-selection and /dashboard directly.

The PR introduces a full auth flow (AuthService, login with PBKDF2+AES, session storage) but never wires it to the router. Both the role-selection route and the entire authenticated shell (path: '') are reachable without credentials.

The canActivate guard 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 using inject() when dependencies are needed.

The most idiomatic fix is:

  1. Add a functional authGuard in core/guards/auth.guard.ts that delegates to AuthService.
  2. Apply canActivate: [authGuard] to the role-selection route.
  3. Apply canActivateChild: [authGuard] on the path: '' 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 win

Empty encKey will break AES encryption at runtime or produce trivially weak ciphertext.

session-storage.service.ts derives SECRET_KEY from environment.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

telephoneServer points to a live UAT host over plain HTTP.

Unlike every other URL in this file (which are all localhost), telephoneServer is 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 to https:// and consider whether a localhost stub 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

SessionStorageService derives its SECRET_KEY from environment.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 in SessionStorageService that 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.5 has 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 xlsx are no longer maintained. CVE-2024-22363 was resolved in 0.20.2, available via the SheetJS CDN.
  • The xlsx package 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

telephoneServer uses plain HTTP against an external domain

All other base URLs target localhost (acceptable for dev), but telephoneServer: '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 to https:// 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 win

Forcing Content-Type: application/json on every request breaks non-JSON uploads

File uploads using multipart/form-data require the browser to set the boundary-encoded Content-Type automatically. Overriding it unconditionally strips the boundary parameter, causing the server to reject the body. Skip the header for requests that already have a Content-Type or carry a FormData/Blob body.

🐛 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-foreground is referenced in @theme but never defined in :root or .dark

Line 93 maps --color-destructive-foreground: var(--destructive-foreground), but --destructive-foreground has no declared value in either :root or .dark. Any component using bg-destructive-foreground or text-destructive-foreground will 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() bypass SessionStorageService, likely returning ciphertext

SessionStorageService applies 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 the Authorization header, 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 through SessionStorageService:

🐛 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 Vitest

The PR description and AI summary confirm Vitest is the configured test runner (referenced in .vscode/launch.json/tasks.json). ng test invokes the Angular CLI test builder (Karma/Jest), not Vitest, so npm test in 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 win

Avoid 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 win

Fail 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 win

Restore keyboard focus visibility and explicit labels for icon buttons

Lines 48-74 remove focus outlines and depend on title. Add aria-label plus visible focus-visible styles 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 win

Harden 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

#isFirstChange toggling forces repeated template view recreation.

On Lines 96-98, #isFirstChange gets set back to true for 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

zTitle type promises TemplateRef, but only string rendering is implemented.

ZardDialogOptions.zTitle allows TemplateRef<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 win

Detect host element type from the host tag, not its parent.

On Lines 128-130, checking parentElement.tagName misclassifies 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 win

Clear authToken during logout to avoid stale Authorization usage.

On Lines 62-63, authToken is left behind in sessionStorage, 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 win

Custom 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 win

Numeric 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 win

Use filtered 104 privileges for gating and role extraction.

On Lines 182-189, you filter to privileges104 but continue with res.previlegeObj[0]. This can admit non-104 users and can throw if roles[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 win

Move withCredentials to HttpClient options instead of request payload.

withCredentials: true on 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.ts and src/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 win

Clear the DataService role state on logout too.

This component populates current_role, current_roleID, current_feature, current_service, screens, and agentID, 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 win

Block navigation when no feature mapping was resolved.

getSelectedFeature() can return '' here, but selectRole() still emits roleSelected and routes to /dashboard with partially updated DataService. That lets a malformed role reuse the previous selection's current_feature / current_role / screens state. 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 win

Pass the caller's ViewContainerRef into TemplatePortal.

Angular CDK's TemplatePortal requires a non-null ViewContainerRef to stamp the template into the correct context. Passing null as unknown as ViewContainerRef at 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. Since zViewContainerRef is 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 win

Replace scaffolded <title> with a user-facing value.

Helpline104V19 is 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 === max causes division-by-zero in convertValueToPercentage, returning NaN.

When min and max are equal the denominator (max - min) is 0, yielding NaN, 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 test debug URL targets Karma, but the project uses Vitest.

http://localhost:9876/debug.html is Karma's debug page. Since tsconfig.spec.json declares vitest/globals as the test type provider, Vitest is the test runner — it does not expose a /debug.html endpoint 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/browser or 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 = 0 causes a division-by-zero, returning NaN.

If step is 0, (value - min) / step produces Infinity or NaN, 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 win

Reorder Tailwind v4 variant chain for file: responsive styles

At Lines 32-34, file:max-md:... violates Tailwind v4 variant ordering. In v4, responsive variants must be applied before pseudo-elements. Use max-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 win

Render zDescription independently of zTitle.

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.

@sindhureddy-6

Copy link
Copy Markdown

Hi @Anurag9699 @drtechie has this issue already been assigned?.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants