MOSIP-25201 - Fix (round 3) accessibility and SonarQube reliability issues in multiple components#224
Conversation
WalkthroughEight Angular components changed lifecycle initialization: Changes
Sequence DiagramsequenceDiagram
participant Browser as Component
participant ngInit as ngOnInit()
participant Init as initAsync() / initializeAfterLogout()
participant Services as Services/API
Note over Browser,Services: Old pattern — ngOnInit awaited async work
Browser->>ngInit: component created
ngInit->>Services: await asyncServiceCalls()
Services-->>ngInit: data
ngInit-->>Browser: initialized
Note over Browser,Services: New pattern — synchronous ngOnInit + async initializer
Browser->>ngInit: component created
ngInit->>Init: invoke (fire & forget)
ngInit-->>Browser: returns immediately
Init->>Services: await asyncServiceCalls()
Services-->>Init: data
Init-->>Browser: initialization completes (async)
Note right of Init: Login uses initializeAfterLogout(forceLogout, langCode) for logout/config sequencing
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
pre-registration-ui/src/app/feature/file-upload/file-upload/file-upload.component.ts (1)
1435-1441: Bug: assignment instead of comparisonThis condition assigns rather than compares, making it always truthy.
- this.users[0].files[0].documentsMetaData.forEach((element) => { - if ((element.docCatCode = category)) { + this.users[0].files[0].documentsMetaData.forEach((element) => { + if (element.docCatCode === category) { return true; } });pre-registration-ui/src/app/feature/booking/time-selection/time-selection.component.ts (1)
235-247: Async bug: forEach + async/await resolves earlygetLocationNamesByCodes() resolves when the last iteration finishes, not when all finish. Use Promise.all or a for..of loop.
- getLocationNamesByCodes() { - return new Promise((resolve) => { - if (this.locationCodes.length == 0) { - resolve(true); - } - this.locationCodes.forEach(async (pins,index) => { - await this.getLocationNames(pins); - if(index===this.locationCodes.length-1){ - resolve(true); - } - }); - }); - } + async getLocationNamesByCodes() { + if (this.locationCodes.length === 0) { return; } + await Promise.all(this.locationCodes.map((code) => this.getLocationNames(code))); + }pre-registration-ui/src/app/feature/summary/acknowledgement/acknowledgement.component.ts (1)
86-107: Add error handling to prevent silent failures.The
initAsync()method contains multiple await calls but lacks error handling. If any operation fails, the error will be silently swallowed (due to the fire-and-forget pattern on lines 67, 72), andshowSpinnerwill never be set tofalse, leaving the UI in a broken state.Apply this diff to add proper error handling:
private async initAsync(): Promise<void> { + try { await this.getUserInfo(this.preRegIds); // console.log(this.usersInfoArr); for (let i = 0; i < this.usersInfoArr.length; i++) { await this.getRegCenterDetails(this.usersInfoArr[i].langCode, i); await this.getLabelDetails(this.usersInfoArr[i].langCode, i); await this.getUserLangLabelDetails(this.langCode, i); } const notificationTypes = this.configService .getConfigByKey(appConstants.CONFIG_KEYS.mosip_notification_type) .split('|'); this.notificationTypes = notificationTypes.map((item) => item.toUpperCase() ); await this.apiCalls(); if (this.bookingService.getSendNotification()) { this.bookingService.resetSendNotification(); await this.automaticNotification(); } this.prepareAckDataForUI(); + } catch (error) { + this.showErrorMessage(error); + } finally { this.showSpinner = false; + } }
🧹 Nitpick comments (6)
pre-registration-ui/src/app/feature/dashboard/dashboard/dashboard.component.ts (3)
108-143: Fire-and-forget init can race with consumers of schema/locationHierarchygetIdentityJsonFormat() is now called without await; any code reading localStorage 'schema' or 'locationHierarchy' during/soon after ngOnInit can see stale/missing values. Consider gating consumers or awaiting once on first load. At minimum, mark a ready flag before downstream use.
If any template or method reads these keys during init, ensure it’s behind a dataLoaded flag or await the call once at startup.
23-23: Remove unused importutf8Encode is imported but never used.
- import { utf8Encode } from "@angular/compiler/src/util";
145-173: Resolve promise on error to avoid hanging callersgetIdentityJsonFormat() never resolves on error. Even if currently fire-and-forget, future awaiters would hang.
this.dataStorageService.getIdentityJson().subscribe( (response) => { // ... resolve(true); }, (error) => { - this.showErrorMessage(error); + this.showErrorMessage(error); + resolve(false); // ensure the promise settles } );pre-registration-ui/src/app/auth/login/login.component.ts (1)
98-99: Don’t set dir before it’s computedlocalStorage 'dir' is set here before setLanguageDirection runs; it may momentarily be wrong. Let loadConfigs/setLanguageDirection update it.
- localStorage.setItem('dir', this.dir);pre-registration-ui/src/app/feature/booking/center-selection/center-selection.component.ts (1)
235-247: Optional: fix async forEach pattern (same issue as time-selection)Use Promise.all or for..of to ensure all getLocationNames complete before proceeding.
- getLocationNamesByCodes() { - return new Promise((resolve) => { - if (this.locationCodes.length == 0) { - resolve(true); - } - this.locationCodes.forEach(async (pins,index) => { - await this.getLocationNames(pins); - if(index===this.locationCodes.length-1){ - resolve(true); - } - }); - }); - } + async getLocationNamesByCodes() { + if (this.locationCodes.length === 0) { return; } + await Promise.all(this.locationCodes.map((code) => this.getLocationNames(code))); + }pre-registration-ui/src/app/feature/summary/acknowledgement/acknowledgement.component.ts (1)
379-446: Consider extracting repeated string normalization to a helper function.The same normalization pattern (stringify → remove brackets → replace commas → remove quotes) is repeated 7 times. This can be refactored into a reusable helper function to improve maintainability and reduce duplication.
Consider adding this helper method to the class:
private normalizeArrayToString(arr: any[]): string { return JSON.stringify(arr) // NOSONAR: Cannot use String.replaceAll() due to Angular 7 browser support; using replace + global regex instead .replace(/\[/g, "") // NOSONAR: Cannot use String.replaceAll() due to Angular 7 browser support; using replace + global regex instead .replace(/,/g, " / ") // NOSONAR: Cannot use String.replaceAll() due to Angular 7 browser support; using replace + global regex instead .replace(/"/g, " ") // NOSONAR: Cannot use String.replaceAll() due to Angular 7 browser support; using replace + global regex instead .replace(/]/g, ""); }Then replace each occurrence:
- this.ackDataItem["preRegIdLabels"] = JSON.stringify( - preRegIdLabels - ) - // NOSONAR: Cannot use String.replaceAll() due to Angular 7 browser support; using replace + global regex instead - .replace(/\[/g, "") - // NOSONAR: Cannot use String.replaceAll() due to Angular 7 browser support; using replace + global regex instead - .replace(/,/g, " / ") - // NOSONAR: Cannot use String.replaceAll() due to Angular 7 browser support; using replace + global regex instead - .replace(/"/g, " ") - // NOSONAR: Cannot use String.replaceAll() due to Angular 7 browser support; using replace + global regex instead - .replace(/]/g, ""); + this.ackDataItem["preRegIdLabels"] = this.normalizeArrayToString(preRegIdLabels);Apply the same pattern for
appDateLabels,contactPhoneLabels,labelNames,nameValues,labelRegCntrs, andregCntrNames.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
pre-registration-ui/src/app/auth/login/login.component.ts(3 hunks)pre-registration-ui/src/app/feature/booking/center-selection/center-selection.component.ts(1 hunks)pre-registration-ui/src/app/feature/booking/time-selection/time-selection.component.ts(1 hunks)pre-registration-ui/src/app/feature/dashboard/dashboard/dashboard.component.ts(2 hunks)pre-registration-ui/src/app/feature/demographic/demographic/demographic.component.ts(4 hunks)pre-registration-ui/src/app/feature/file-upload/file-upload/file-upload.component.ts(2 hunks)pre-registration-ui/src/app/feature/summary/acknowledgement/acknowledgement.component.ts(2 hunks)pre-registration-ui/src/app/feature/summary/preview/preview.component.ts(4 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build-pre-registration-ui / npm-build
🔇 Additional comments (8)
pre-registration-ui/src/app/feature/file-upload/file-upload/file-upload.component.ts (1)
130-141: ngOnInit/initAsync split looks goodSynchronous ngOnInit with a deferred initAsync is fine and improves determinism.
pre-registration-ui/src/app/feature/summary/preview/preview.component.ts (2)
71-84: Synchronous ngOnInit with deferred init is fine; ensure template guardsinitAsync runs unattended; keep using dataLoaded guards in the template to prevent ExpressionChanged errors or partial renders.
385-397: Good: replaceAll avoidance with NOSONARRegex-based replace for Angular 7 compatibility is appropriate.
pre-registration-ui/src/app/auth/login/login.component.ts (1)
311-328: Good: use Number.isNaN guardThis removes false positives from global isNaN coercion.
pre-registration-ui/src/app/feature/demographic/demographic/demographic.component.ts (2)
190-201: ngOnInit/initAsync split looks goodPattern aligns with other components. Keep template guarded by data/state flags.
298-321: Good: regex replace with NOSONAR instead of replaceAllMaintains Angular 7/browser compatibility.
Also applies to: 302-304, 306-312, 314-320
pre-registration-ui/src/app/feature/booking/center-selection/center-selection.component.ts (1)
97-101: Good: numeric conversion for config valueCasting recommendedCenterLocCode to number prevents string comparisons.
pre-registration-ui/src/app/feature/summary/acknowledgement/acknowledgement.component.ts (1)
64-65: Good defensive coding for localStorage access.The code properly handles the case where
localStorage.getItem('multiappointment')might returnnull, providing a safe fallback to an empty array.
pre-registration-ui/src/app/feature/booking/center-selection/center-selection.component.ts
Show resolved
Hide resolved
pre-registration-ui/src/app/feature/booking/time-selection/time-selection.component.ts
Outdated
Show resolved
Hide resolved
pre-registration-ui/src/app/feature/booking/time-selection/time-selection.component.ts
Outdated
Show resolved
Hide resolved
pre-registration-ui/src/app/feature/booking/time-selection/time-selection.component.ts
Outdated
Show resolved
Hide resolved
pre-registration-ui/src/app/feature/file-upload/file-upload/file-upload.component.ts
Outdated
Show resolved
Hide resolved
pre-registration-ui/src/app/feature/summary/acknowledgement/acknowledgement.component.ts
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (1)
pre-registration-ui/src/app/feature/booking/time-selection/time-selection.component.ts (1)
89-114: Critical: multiappointment path never setsregistrationCenter.The multiappointment branch (line 90) calls
initAsync()immediately, butregistrationCenteris never populated. The defensive guard at line 117 will causeinitAsync()to return early, leaving the component in an incomplete state.In contrast, the single-appointment path (lines 97-101) sets
registrationCenterfrom route params before callinginitAsync().Based on the past review comments,
registrationCentershould come fromqueryParams. Add aqueryParamssubscription that setsregistrationCenterand triggers initialization only after bothpreRegIdandregistrationCenterare known:ngOnInit(): void { if (this.router.url.includes('multiappointment')) { const stored = localStorage.getItem('multiappointment'); this.preRegId = stored ? [...JSON.parse(stored)] : []; - // we already know preRegId, we can start async init - void this.initAsync(); } else { - // we don't know preRegId yet - wait for params then start async init this.activatedRoute.params.subscribe((param) => { this.preRegId = [param['appId']]; - this.registrationCenter = param['regCenter']; - void this.initAsync(); + if (this.registrationCenter) { void this.initAsync(); } }); } + this.activatedRoute.queryParams.subscribe((param) => { + this.registrationCenter = param['regCenter']; + if (this.preRegId && this.preRegId.length) { void this.initAsync(); } + }); this.name = this.configService.getConfigByKey( appConstants.CONFIG_KEYS.preregistration_identity_name );
🧹 Nitpick comments (4)
pre-registration-ui/src/app/feature/file-upload/file-upload/file-upload.component.ts (1)
130-140: Consider adding error handling for the fire-and-forget async initialization.Making
ngOnInitsynchronous is good practice. However, the fire-and-forget pattern (void this.initAsync()) means any errors thrown ininitAsyncwill be silently swallowed, making debugging difficult.Consider adding error handling:
- // async part moved to helper; fire and forget - void this.initAsync(); + // async part moved to helper; log errors if initialization fails + this.initAsync().catch(err => { + this.loggerService.error('Initialization failed', err); + this.showErrorMessage(err); + });pre-registration-ui/src/app/feature/booking/center-selection/center-selection.component.ts (2)
106-107: Remove duplicate console.log statement.The same log statement appears twice consecutively.
Apply this diff:
this.allLocationTypes = response[appConstants.RESPONSE]['locationHierarchyLevels']; console.log(this.allLocationTypes); - console.log(this.allLocationTypes); //get the recommended loc hierachy code to which booking centers are mapped
91-121: Consider adding error handling to initAsync.Since
initAsync()is invoked with thevoidoperator, any exceptions thrown during initialization (e.g., fromgetUserInfoorgetIdentityJsonFormat) may fail silently without user feedback. Consider wrapping the body in a try-catch block to handle errors gracefully and show appropriate error messages.Example pattern:
private async initAsync(): Promise<void> { try { this.getErrorLabels(); await this.getUserInfo(this.preRegId); // ... rest of initialization } catch (error) { console.error('Initialization failed:', error); this.showErrorMessage(error); } }pre-registration-ui/src/app/feature/summary/acknowledgement/acknowledgement.component.ts (1)
380-447: Refactor repetitive string transformations into a helper function.The label transformation logic is duplicated seven times, with 28 identical NOSONAR comments. This addresses SonarQube warnings but creates maintenance burden.
Extract the transformation into a reusable helper method:
Add this helper method to the class:
/** * Transforms a label array into a formatted string. * Uses replace() with global regex instead of replaceAll() for Angular 7 browser support. */ private formatLabelsForDisplay(labels: string[]): string { return JSON.stringify(labels) .replace(/\[/g, '') .replace(/,/g, ' / ') .replace(/"/g, ' ') .replace(/]/g, ''); }Then refactor the prepareAckDataForUI method to use it:
- this.ackDataItem["preRegIdLabels"] = JSON.stringify( - preRegIdLabels - ) - // NOSONAR: Cannot use String.replaceAll() due to Angular 7 browser support; using replace + global regex instead - .replace(/\[/g, "") - // NOSONAR: Cannot use String.replaceAll() due to Angular 7 browser support; using replace + global regex instead - .replace(/,/g, " / ") - // NOSONAR: Cannot use String.replaceAll() due to Angular 7 browser support; using replace + global regex instead - .replace(/"/g, " ") - // NOSONAR: Cannot use String.replaceAll() due to Angular 7 browser support; using replace + global regex instead - .replace(/]/g, ""); + this.ackDataItem["preRegIdLabels"] = this.formatLabelsForDisplay(preRegIdLabels);Apply the same pattern to all seven label fields:
appDateLabels,contactPhoneLabels,labelNames,nameValues,labelRegCntrs, andregCntrNames.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
pre-registration-ui/src/app/auth/login/login.component.ts(3 hunks)pre-registration-ui/src/app/feature/booking/center-selection/center-selection.component.ts(1 hunks)pre-registration-ui/src/app/feature/booking/time-selection/time-selection.component.ts(1 hunks)pre-registration-ui/src/app/feature/file-upload/file-upload/file-upload.component.ts(3 hunks)pre-registration-ui/src/app/feature/summary/acknowledgement/acknowledgement.component.ts(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build-pre-registration-ui / npm-build
🔇 Additional comments (9)
pre-registration-ui/src/app/feature/file-upload/file-upload/file-upload.component.ts (2)
142-161: Async initialization logic is well-structured.The extraction of async initialization into a dedicated method improves code organization and makes
ngOnInitsynchronous, which aligns with Angular best practices.Note: Ensure the caller handles errors appropriately (see comment on lines 130-140).
950-952: Extension validation properly fixed.The refactored regex construction correctly addresses the previous review concern:
- Extensions are split, trimmed, and filtered to remove empty values
- Regex is anchored (
^...$) to match exact extensions rather than substrings- Returns
nullwhen no extensions exist, preventing the match-all behavior- Null guard on line 962 ensures safe testing
Based on past review comments.
Also applies to: 962-962
pre-registration-ui/src/app/feature/booking/center-selection/center-selection.component.ts (1)
78-89: Race condition fix looks good.The previous review flagged that
initAsynccould run beforepreRegIdwas populated. The current implementation correctly addresses this by callinginitAsync()only afterpreRegIdis assigned in both the multiappointment and route params branches.pre-registration-ui/src/app/feature/booking/time-selection/time-selection.component.ts (1)
116-128: LGTM: defensive guards properly implemented.The early return at line 117 prevents backend calls with an undefined
registrationCenter, and the comprehensive guard at lines 124-126 prevents the "Cannot read property 'split' of null/undefined" error previously flagged.pre-registration-ui/src/app/feature/summary/acknowledgement/acknowledgement.component.ts (2)
75-81: Memory leak fix confirmed.The subscription is now properly tracked in the
subscriptionsarray, resolving the memory leak flagged in the previous review.
87-108: Well-structured async initialization flow.The refactor successfully extracts async work from
ngOnInit()into a dedicated method. The sequential awaits ensure proper initialization order, and the spinner state is correctly managed.pre-registration-ui/src/app/auth/login/login.component.ts (3)
78-79: Guarding navigation when langCode is availableThanks for adding the langCode truthiness check before building the dashboard URL; this removes the null-segment navigation gap highlighted earlier.
157-158: Safer NaN guard in OTP reload flowThe switch to
Number.isNaNavoids the coercion pitfalls of the globalisNaN, which keeps the expiry logic honest.
311-312: Consistent NaN protection during timer setupUsing
Number.isNaNhere keeps the timer’s defaulting logic aligned with the reload path—nice consistency improvement.
pre-registration-ui/src/app/feature/booking/center-selection/center-selection.component.ts
Outdated
Show resolved
Hide resolved
pre-registration-ui/src/app/feature/booking/center-selection/center-selection.component.ts
Outdated
Show resolved
Hide resolved
pre-registration-ui/src/app/feature/booking/time-selection/time-selection.component.ts
Outdated
Show resolved
Hide resolved
pre-registration-ui/src/app/feature/summary/acknowledgement/acknowledgement.component.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pre-registration-ui/src/app/feature/summary/preview/preview.component.ts (1)
71-83: Add error handling for fire-and-forget async initialization.The
initAsync()call is fire-and-forget without error handling. If initialization fails, the error becomes an unhandled promise rejection anddataLoadedremains false indefinitely, leaving the UI in a loading state.Attach explicit rejection handling:
ngOnInit() { this.activatedRoute.params.subscribe((param) => { this.preRegId = param["appId"]; }); if (this.ltrLangs.includes(this.userPrefLanguage)) { this.userPrefLanguageDir = "ltr"; } else { this.userPrefLanguageDir = "rtl"; } this.getLanguageLabels(); // kick off async initialisation (we ignore the returned Promise on purpose) - void this.initAsync(); + this.initAsync().catch(err => { + console.error('Initialization failed:', err); + this.dataLoaded = true; // allow UI to render in error state + }); }
♻️ Duplicate comments (1)
pre-registration-ui/src/app/auth/login/login.component.ts (1)
89-99: Config loading race condition still exists.The previous review identified that
loadConfigs()doesn't wait for the config to be set beforehandleBrowserReload()runs. While error handling was added, the underlying race condition remains:handleBrowserReload()will execute with potentially empty/default config values.Based on past review comments
The fix from the previous review was not fully applied. Change
loadConfigs()to return a Promise:- loadConfigs(): void { - this.dataService.getConfig().subscribe((response) => { + async loadConfigs(): Promise<void> { + return new Promise((resolve) => { + this.dataService.getConfig().subscribe((response) => { this.configService.setConfig(response); this.appVersion = this.configService.getConfigByKey( "preregistration.ui.version" ); // ... rest of config setup this.showSpinner = false; + resolve(); }); + }); }This ensures
await this.loadConfigs()on line 94 actually waits for the config to be loaded.
🧹 Nitpick comments (2)
pre-registration-ui/src/app/feature/summary/preview/preview.component.ts (1)
85-162: LGTM with a suggestion: Consider adding early-exit validation.The async initialization sequence is well-structured with proper await chains.
Consider adding validation after critical steps to fail fast:
private async initAsync(): Promise<void> { await this.getIdentityJsonFormat(); + if (!this.identityData || this.identityData.length === 0) { + throw new Error('Failed to load identity data'); + } // await this.getResidentDetails(); // await this.getGenderDetails(); await this.getDynamicFieldValues(null); await this.getUserInfo(); + if (!this.user || !this.user.request) { + throw new Error('Failed to load user info'); + } await this.getUserFiles();pre-registration-ui/src/app/feature/booking/center-selection/center-selection.component.ts (1)
78-97: Add error handling for fire-and-forget async initialization.The
initAsync()calls lack error handling. Consider adding rejection handling for better error resilience.} - void this.initAsync(); // start only after preRegId is known + this.initAsync().catch(err => { + console.error('Initialization failed:', err); + this.pageLoaded = true; + }); // start only after preRegId is known } else { this.activatedRoute.params.subscribe((param) => { this.preRegId = [param['appId']]; - void this.initAsync(); // start only after param is known + this.initAsync().catch(err => { + console.error('Initialization failed:', err); + this.pageLoaded = true; + }); // start only after param is known }); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
pre-registration-ui/src/app/auth/login/login.component.ts(4 hunks)pre-registration-ui/src/app/feature/booking/center-selection/center-selection.component.ts(1 hunks)pre-registration-ui/src/app/feature/booking/time-selection/time-selection.component.ts(1 hunks)pre-registration-ui/src/app/feature/demographic/demographic/demographic.component.ts(4 hunks)pre-registration-ui/src/app/feature/summary/acknowledgement/acknowledgement.component.ts(2 hunks)pre-registration-ui/src/app/feature/summary/preview/preview.component.ts(4 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build-pre-registration-ui / npm-build
🔇 Additional comments (12)
pre-registration-ui/src/app/feature/summary/preview/preview.component.ts (1)
386-386: NOSONAR comment is appropriate.The justification for using
replace()with a global regex instead ofreplaceAll()for Angular 7 browser compatibility is valid.pre-registration-ui/src/app/feature/demographic/demographic/demographic.component.ts (2)
194-234: Initialization sequence is well-orchestrated.The initialization flow properly sequences config loading, language setup, form initialization, and conditional consent dialogs. The synchronous lifecycle hook pattern is correctly implemented.
298-319: NOSONAR comments are justified.All NOSONAR comments correctly explain why
replace()with global regex is used instead ofString.replaceAll()for Angular 7 browser compatibility.pre-registration-ui/src/app/feature/booking/time-selection/time-selection.component.ts (2)
89-118: Previous race condition concerns addressed.The initialization flow now ensures
preRegIdandregistrationCenterare set before callinginitAsync(). The JSON.parse is properly guarded with try-catch.Based on past review comments
120-134: Good defensive programming with registrationCenter guard.The early return when
registrationCenteris missing prevents the race condition identified in previous reviews. The lunchEndTime null guard also properly prevents the "Cannot read property 'split' of null/undefined" error.Based on past review comments
pre-registration-ui/src/app/auth/login/login.component.ts (2)
75-87: Good guard for langCode before navigation.The addition of
&& langCodein the navigation guard prevents navigating with null/empty segments, addressing the previous review concern.Based on past review comments
157-159: Correct usage of Number.isNaN().Using
Number.isNaN()instead ofisNaN()is the proper approach, as it doesn't perform type coercion and gives more reliable results for numeric validation.Also applies to: 310-328
pre-registration-ui/src/app/feature/booking/center-selection/center-selection.component.ts (2)
78-98: Previous race condition and JSON.parse concerns properly addressed.The initialization flow now ensures
preRegIdis set beforeinitAsync()is called, and the JSON.parse is wrapped in proper error handling with fallback to empty array.Based on past review comments
105-112: Good NaN validation with fallback.The addition of
Number.isNaN()check with a sensible default value (1) prevents the filter comparison issues identified in previous reviews. The error logging helps with debugging.Based on past review comments
pre-registration-ui/src/app/feature/summary/acknowledgement/acknowledgement.component.ts (3)
62-92: All previous concerns properly addressed.The initialization properly handles:
- JSON.parse errors with try-catch and fallback
- Error handling for the fire-and-forget async call with spinner state cleanup
- Subscription tracking for I18N language files
Excellent implementation of the recommendations from previous reviews.
Based on past review comments
94-115: Well-structured async initialization sequence.The initialization properly sequences user info loading, registration center details, label fetching, and notification setup. The logic is clear and maintainable.
390-454: NOSONAR comments are appropriate and consistent.All NOSONAR comments correctly explain the browser compatibility constraint requiring
replace()with global regex instead ofString.replaceAll().
pre-registration-ui/src/app/feature/booking/time-selection/time-selection.component.ts
Show resolved
Hide resolved
pre-registration-ui/src/app/feature/demographic/demographic/demographic.component.ts
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pre-registration-ui/src/app/feature/summary/preview/preview.component.ts (1)
71-83: Add error handling for fire-and-forget async initialization.The
initAsync()call lacks error handling. If initialization fails during data fetching or processing, the component will remain in an incomplete state (dataLoadedstaysfalse) without providing user feedback.Add rejection handling:
ngOnInit() { this.activatedRoute.params.subscribe((param) => { this.preRegId = param["appId"]; }); if (this.ltrLangs.includes(this.userPrefLanguage)) { this.userPrefLanguageDir = "ltr"; } else { this.userPrefLanguageDir = "rtl"; } this.getLanguageLabels(); // kick off async initialisation (we ignore the returned Promise on purpose) - void this.initAsync(); + this.initAsync().catch(err => { + console.error('Initialization failed:', err); + this.dataLoaded = false; + this.showErrorMessage(err); + }); }
♻️ Duplicate comments (1)
pre-registration-ui/src/app/feature/demographic/demographic/demographic.component.ts (1)
190-193: Add error handling for fire-and-forget async initialization.Similar to previous reviews, the
initAsync()call lacks error handling. If initialization fails during form setup or config loading, the component will remain in an incomplete state (primaryuserFormstaysfalse) without user feedback.Add rejection handling:
ngOnInit(): void { - void this.initAsync(); + this.initAsync().catch(err => { + console.error('Initialization failed:', err); + this.primaryuserForm = false; + this.showErrorMessage(err); + }); }
🧹 Nitpick comments (4)
pre-registration-ui/src/app/feature/booking/center-selection/center-selection.component.ts (2)
78-98: Consider consistent error handling for both initialization paths.The multiappointment path has error handling for
initAsync()(line 71), but the route-param path does not (line 95). While this may be acceptable in "Chill" mode, consistency would improve maintainability.Apply this diff for consistency:
} else { this.activatedRoute.params.subscribe((param) => { this.preRegId = [param['appId']]; - void this.initAsync(); // start only after param is known + this.initAsync().catch(error => { + console.error('Initialization failed:', error); + this.pageLoaded = true; + }); }); }
115-133: Remove duplicate console.log statement.Line 119 duplicates the logging statement from line 118.
Apply this diff:
//get all location types from db this.allLocationTypes = response[appConstants.RESPONSE]['locationHierarchyLevels']; console.log(this.allLocationTypes); - console.log(this.allLocationTypes); //get the recommended loc hierachy code to which booking centers are mappedpre-registration-ui/src/app/feature/summary/acknowledgement/acknowledgement.component.ts (2)
62-92: Consider consistent error handling for both initialization paths.The multiappointment path includes error handling for
initAsync()(lines 71-74), but the route-param path does not (line 79). For consistency and robustness, both paths should handle initialization failures.Apply this diff:
} else { // wait for appId from route before starting async init this.activatedRoute.params.subscribe((param) => { this.preRegIds = [param['appId']]; - void this.initAsync(); + this.initAsync().catch(error => { + console.error('Initialization failed:', error); + this.showSpinner = false; + }); }); }
387-454: Consider consolidating repetitive NOSONAR comments.While the NOSONAR justification is valid, repeating the identical comment on every
.replace()call in each chain is verbose. Consider placing a single comment before each chain.Example refactoring for one chain:
+ // NOSONAR: Cannot use String.replaceAll() due to Angular 7 browser support; using replace + global regex instead this.ackDataItem["preRegIdLabels"] = JSON.stringify( preRegIdLabels ) - // NOSONAR: Cannot use String.replaceAll() due to Angular 7 browser support; using replace + global regex instead .replace(/\[/g, "") - // NOSONAR: Cannot use String.replaceAll() due to Angular 7 browser support; using replace + global regex instead .replace(/,/g, " / ") - // NOSONAR: Cannot use String.replaceAll() due to Angular 7 browser support; using replace + global regex instead .replace(/"/g, " ") - // NOSONAR: Cannot use String.replaceAll() due to Angular 7 browser support; using replace + global regex instead .replace(/]/g, "");Apply similar refactoring to the other chains (appDateLabels, contactPhoneLabels, etc.).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
pre-registration-ui/src/app/auth/login/login.component.ts(4 hunks)pre-registration-ui/src/app/feature/booking/center-selection/center-selection.component.ts(1 hunks)pre-registration-ui/src/app/feature/booking/time-selection/time-selection.component.ts(2 hunks)pre-registration-ui/src/app/feature/dashboard/dashboard/dashboard.component.ts(2 hunks)pre-registration-ui/src/app/feature/demographic/demographic/demographic.component.ts(4 hunks)pre-registration-ui/src/app/feature/file-upload/file-upload/file-upload.component.ts(3 hunks)pre-registration-ui/src/app/feature/summary/acknowledgement/acknowledgement.component.ts(2 hunks)pre-registration-ui/src/app/feature/summary/preview/preview.component.ts(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- pre-registration-ui/src/app/feature/file-upload/file-upload/file-upload.component.ts
- pre-registration-ui/src/app/feature/booking/time-selection/time-selection.component.ts
- pre-registration-ui/src/app/feature/dashboard/dashboard/dashboard.component.ts
- pre-registration-ui/src/app/auth/login/login.component.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build-pre-registration-ui / npm-build
🔇 Additional comments (6)
pre-registration-ui/src/app/feature/summary/preview/preview.component.ts (2)
85-162: LGTM!The async initialization logic is well-structured with proper sequencing of asynchronous operations. The method correctly orchestrates data fetching and sets the
dataLoadedflag upon completion.
385-397: LGTM!The NOSONAR comment appropriately justifies the use of
replace()with global regex instead ofreplaceAll()for Angular 7 browser compatibility.pre-registration-ui/src/app/feature/demographic/demographic/demographic.component.ts (2)
194-234: LGTM!The async initialization method properly orchestrates the component setup sequence, including language initialization, form setup, and conditional consent flow.
298-320: LGTM!The NOSONAR comments consistently and appropriately justify the use of
replace()with global regex patterns for Angular 7 compatibility throughout the date format localization logic.pre-registration-ui/src/app/feature/booking/center-selection/center-selection.component.ts (1)
105-112: LGTM!The NaN validation properly addresses the previous review concern by safely parsing the config value, validating with
Number.isNaN(), and providing a sensible fallback. This prevents downstream comparison issues.pre-registration-ui/src/app/feature/summary/acknowledgement/acknowledgement.component.ts (1)
94-115: LGTM!The async initialization properly orchestrates user info fetching, registration center details, label loading, and notification setup. The sequential processing ensures data is available when needed.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
pre-registration-ui/src/app/feature/booking/time-selection/time-selection.component.ts (1)
94-126: Harden JSON.parse and resolve race condition with registrationCenter.Two reliability concerns:
- Line 95:
JSON.parsewill throw if localStorage contains malformed JSON.- Lines 101-103 vs 125: The
queryParamssubscription setsregistrationCenterasynchronously, butinitAsynccontinues synchronously and callsgetSlotsforCenter(this.registrationCenter)at line 125. If the subscription hasn't fired yet,registrationCenterwill be undefined.Apply this diff to fix both issues:
private async initAsync(): Promise<void> { if (this.router.url.includes("multiappointment")) { - this.preRegId = [...JSON.parse(localStorage.getItem("multiappointment"))]; + const stored = localStorage.getItem("multiappointment"); + try { + this.preRegId = stored ? [...JSON.parse(stored)] : []; + } catch { + this.preRegId = []; + } } else { this.activatedRoute.params.subscribe((param) => { this.preRegId = [param["appId"]]; }); } - this.activatedRoute.queryParams.subscribe((param) => { - this.registrationCenter = param["regCenter"]; - }); + // Wait for registrationCenter before proceeding + await new Promise<void>(resolve => { + this.activatedRoute.queryParams.subscribe((param) => { + this.registrationCenter = param["regCenter"]; + resolve(); + }); + }); + if (!this.registrationCenter) { + console.error('registrationCenter is missing'); + this.spinner = false; + return; + } this.name = this.configService.getConfigByKey(pre-registration-ui/src/app/feature/summary/preview/preview.component.ts (1)
71-83: Add error handling for fire-and-forget async initialization.The
initAsync()call at line 82 has no rejection handling. If initialization fails (e.g., network error, API failure), the component will be in an incomplete state without user feedback, anddataLoadedwill never become true.Apply this diff:
this.getLanguageLabels(); - // kick off async initialisation (we ignore the returned Promise on purpose) - void this.initAsync(); + this.initAsync().catch(err => { + console.error('Initialization failed:', err); + this.dataLoaded = false; + this.showErrorMessage(err); + }); }
♻️ Duplicate comments (1)
pre-registration-ui/src/app/feature/booking/time-selection/time-selection.component.ts (1)
89-91: Add error handling for fire-and-forget async initialization.The fire-and-forget call to
initAsync()lacks rejection handling. If initialization fails, users won't see an error and the spinner may remain visible indefinitely.Apply this diff:
ngOnInit(): void { - void this.initAsync(); + this.initAsync().catch(err => { + console.error('Initialization failed:', err); + this.spinner = false; + this.showErrorMessage(err); + }); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
pre-registration-ui/src/app/auth/login/login.component.ts(5 hunks)pre-registration-ui/src/app/feature/booking/center-selection/center-selection.component.ts(1 hunks)pre-registration-ui/src/app/feature/booking/time-selection/time-selection.component.ts(2 hunks)pre-registration-ui/src/app/feature/dashboard/dashboard/dashboard.component.ts(2 hunks)pre-registration-ui/src/app/feature/demographic/demographic/demographic.component.ts(4 hunks)pre-registration-ui/src/app/feature/file-upload/file-upload/file-upload.component.ts(3 hunks)pre-registration-ui/src/app/feature/summary/acknowledgement/acknowledgement.component.ts(2 hunks)pre-registration-ui/src/app/feature/summary/preview/preview.component.ts(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- pre-registration-ui/src/app/feature/dashboard/dashboard/dashboard.component.ts
- pre-registration-ui/src/app/feature/booking/center-selection/center-selection.component.ts
- pre-registration-ui/src/app/feature/file-upload/file-upload/file-upload.component.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build-pre-registration-ui / npm-build
🔇 Additional comments (2)
pre-registration-ui/src/app/feature/demographic/demographic/demographic.component.ts (1)
190-237: LGTM! Correct error handling pattern.The implementation properly handles initialization errors by catching rejections, logging them, and setting
primaryuserFormtofalseto prevent form rendering in an incomplete state. TheinitAsync()method correctly orchestrates the sequential initialization steps withawait.pre-registration-ui/src/app/auth/login/login.component.ts (1)
76-138: LGTM! Initialization refactoring addresses previous concerns.The implementation correctly:
- Guards navigation (line 79): Only navigates when
langCodeis truthy, preventing null route segments.- Handles errors (line 83): Catches initialization failures and logs them.
- Awaits config (lines 111-138):
loadConfigs()now properly awaits the config response usingpipe(first()).toPromise(), ensuringhandleBrowserReload()runs after the config is applied.- Uses
Number.isNaN()(lines 157, 310): Correctly checks for NaN values.The refactoring successfully resolves the race condition where
handleBrowserReload()would execute before config was available.
pre-registration-ui/src/app/feature/summary/acknowledgement/acknowledgement.component.ts
Show resolved
Hide resolved
…ssues in multiple components Signed-off-by: Youssef MAHTAT <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (8)
pre-registration-ui/src/app/feature/dashboard/dashboard/dashboard.component.ts (2)
145-173: Promise never settles on error; track subscriptiongetIdentityJsonFormat() awaits an Http call but never resolves/rejects on error, risking hung flows; also the subscription isn't added to this.subscriptions.
async getIdentityJsonFormat() { return new Promise((resolve, reject) => { - this.dataStorageService.getIdentityJson().subscribe( + const subs = this.dataStorageService.getIdentityJson().subscribe( (response) => { @@ resolve(true); }, (error) => { this.showErrorMessage(error); + resolve(false); // or: reject(error) } ); + this.subscriptions.push(subs); }); }
374-376: Fix NPE: identityObj.langCode read when identityObj is falsyElse-if references identityObj.langCode but identityObj is falsy by definition here; this will throw.
- } else if (identityObj.langCode) { - dataAvailableLanguages = [identityObj.langCode]; + } else if (preregData && preregData.langCode) { + dataAvailableLanguages = [preregData.langCode]; }pre-registration-ui/src/app/feature/summary/preview/preview.component.ts (1)
85-163: Awaited Promises can hang on error (no resolve/reject)getIdentityJsonFormat, getDynamicFieldValues, getUserInfo, and getDocumentCategories never settle on error → initAsync can hang indefinitely.
async getIdentityJsonFormat() { return new Promise((resolve, reject) => { - this.dataStorageService.getIdentityJson().subscribe((response) => { + const subs = this.dataStorageService.getIdentityJson().subscribe((response) => { @@ - }, - (error) => { - this.showErrorMessage(error); - }); + }, (error) => { + this.showErrorMessage(error); + resolve(false); // or reject(error) + }); + this.subscriptions.push(subs); }); } @@ getUserInfo() { return new Promise((resolve) => { - this.dataStorageService + const subs = this.dataStorageService .getUser(this.preRegId.toString()) - .subscribe((response) => { + .subscribe((response) => { @@ - (error) => { - this.showErrorMessage(error); - }); + (error) => { + this.showErrorMessage(error); + resolve(false); + }); + this.subscriptions.push(subs); }); } @@ getDocumentCategories() { const applicantcode = localStorage.getItem("applicantType"); return new Promise((resolve) => { - this.dataStorageService + const subs = this.dataStorageService .getDocumentCategories(applicantcode) - .subscribe((response) => { + .subscribe((response) => { @@ - (error) => { - this.showErrorMessage(error); - }); + (error) => { + this.showErrorMessage(error); + resolve(false); + }); + this.subscriptions.push(subs); }); } @@ async getDynamicFieldValues(pageNo) { @@ - (error) => { - this.showErrorMessage(error); - }) + (error) => { + this.showErrorMessage(error); + resolve(false); + })pre-registration-ui/src/app/feature/demographic/demographic/demographic.component.ts (2)
197-237: Ensure awaited helpers settle on error to avoid hanging initinitAsync awaits methods (getIdentityJsonFormat, getConsentMessage, getDynamicFieldValues, setFormControlValues path uses more) whose error paths don’t resolve/reject.
- await this.getIdentityJsonFormat(); + const idOk = await this.getIdentityJsonFormat(); + if (idOk === false) { this.primaryuserForm = false; return; } @@ - await this.getConsentMessage(); + const consentOk = await this.getConsentMessage(); + if (consentOk === false) { /* optional: proceed without consent */ }And update the helpers:
async getIdentityJsonFormat() { return new Promise((resolve, reject) => { - this.dataStorageService.getIdentityJson().subscribe( + const subs = this.dataStorageService.getIdentityJson().subscribe( @@ - (error) => { - this.showErrorMessage(error); - } + (error) => { + this.showErrorMessage(error); + resolve(false); // or reject(error) + } ); + this.subscriptions.push(subs); }); } @@ private getConsentMessage() { return new Promise((resolve, reject) => { - this.subscriptions.push( - this.dataStorageService.getGuidelineTemplate("consent").subscribe( + this.subscriptions.push( + this.dataStorageService.getGuidelineTemplate("consent").subscribe( (response) => { @@ - (error) => { - this.isConsentMessage = false; - this.showErrorMessage(error); - } + (error) => { + this.isConsentMessage = false; + this.showErrorMessage(error); + resolve(false); // or reject(error) + } ) ); }); } @@ async getDynamicFieldValues(pageNo) { @@ - (error) => { - this.showErrorMessage(error); - }) + (error) => { + this.showErrorMessage(error); + resolve(false); + })
388-408: Consent message fetch should resolve on errorCurrently the Promise can hang when the HTTP call errors; settle the Promise as shown above to keep initAsync from deadlocking.
pre-registration-ui/src/app/feature/file-upload/file-upload/file-upload.component.ts (2)
163-181: Settle Promise on error and track subscriptiongetIdentityJsonFormat never resolves on error; awaiting it can hang; also track the subscription.
async getIdentityJsonFormat() { return new Promise((resolve) => { - this.dataStorageService.getIdentityJson().subscribe((response) => { + const subs = this.dataStorageService.getIdentityJson().subscribe((response) => { @@ - }, - (error) => { - this.showErrorMessage(error); - }); + }, (error) => { + this.showErrorMessage(error); + resolve(false); + }); + this.subscriptions.push(subs); }); }
235-262: Same issue for getUserInfo: settle on errorAwaiters of getUserInfo() may hang on error. Resolve or reject in the error path.
getUserInfo() { return new Promise((resolve) => { - this.dataStorageService + const subs = this.dataStorageService .getUser(this.preRegId.toString()) .subscribe((response) => { @@ - (error) => { - this.showErrorMessage(error); - }); + (error) => { + this.showErrorMessage(error); + resolve(false); + }); + this.subscriptions.push(subs); }); }pre-registration-ui/src/app/feature/summary/acknowledgement/acknowledgement.component.ts (1)
120-205: Fix race: resolve getUserInfo only after all async fetches completeThe Promise resolves when the last index iterates, not when all async tasks finish; data may be incomplete.
- getUserInfo(preRegIds: string[]) { - return new Promise((resolve) => { - preRegIds.forEach(async (prid: any, index) => { - await this.getUserDetails(prid).then(async (user) => { - // ... inner async work - }); - if (index === preRegIds.length - 1) { - resolve(true); - } - }); - }); - } + getUserInfo(preRegIds: string[]) { + return Promise.all( + preRegIds.map(async (prid: any) => { + const user = await this.getUserDetails(prid); + // ... do appointment + push usersInfoArr/applicantContactDetails + }) + ).then(() => true).catch(() => false); + }
♻️ Duplicate comments (2)
pre-registration-ui/src/app/feature/booking/time-selection/time-selection.component.ts (2)
89-104: Start async init only after both preRegId and registrationCenter are known; add rejection handlingCurrent flow can run before queryParams arrive, causing undefined registrationCenter in downstream calls; also missing JSON.parse guard and .catch.
- ngOnInit(): void { - void this.initAsync(); - } + ngOnInit(): void { + if (this.router.url.includes('multiappointment')) { + const stored = localStorage.getItem('multiappointment'); + try { + this.preRegId = stored ? [...JSON.parse(stored)] : []; + } catch { + this.preRegId = []; + } + } else { + this.activatedRoute.params.subscribe((param) => { + this.preRegId = [param['appId']]; + if (this.registrationCenter) { this.initAsync().catch(e => { console.error('Initialization failed:', e); this.spinner = false; }); } + }); + } + this.activatedRoute.queryParams.subscribe((param) => { + this.registrationCenter = param['regCenter']; + if (this.preRegId && this.preRegId.length) { this.initAsync().catch(e => { console.error('Initialization failed:', e); this.spinner = false; }); } + }); + }
121-126: Null‑guard lunchEndTime and gate slot loadingAvoid split() on null/undefined and don’t call backend with undefined center id.
- if (this.temp[0]) { - this.registrationCenterLunchTime = - this.temp[0].registrationCenter.lunchEndTime.split(":"); - } - this.getSlotsforCenter(this.registrationCenter); + if (this.temp && this.temp[0] && this.temp[0].registrationCenter && this.temp[0].registrationCenter.lunchEndTime) { + this.registrationCenterLunchTime = this.temp[0].registrationCenter.lunchEndTime.split(':'); + } else { + this.registrationCenterLunchTime = []; + } + if (this.registrationCenter) { + this.getSlotsforCenter(this.registrationCenter); + }
🧹 Nitpick comments (6)
pre-registration-ui/src/app/feature/dashboard/dashboard/dashboard.component.ts (1)
108-143: Guard fire‑and‑forget init and declare return typeMake ngOnInit explicit and avoid silent failures by catching async errors.
- ngOnInit() { + ngOnInit(): void { @@ - this.getIdentityJsonFormat(); + void this.getIdentityJsonFormat().catch(err => { + this.loggerService.error('getIdentityJsonFormat failed', err); + });pre-registration-ui/src/app/feature/summary/preview/preview.component.ts (2)
71-83: Add rejection handling to fire‑and‑forget initPrevent unhandled promise rejections and surface failures.
- // kick off async initialisation (we ignore the returned Promise on purpose) - void this.initAsync(); + // kick off async initialisation + this.initAsync().catch(err => { + console.error('Preview init failed:', err); + });
188-196: Harden JSON.parse for localStorage readMalformed data will throw and break init.
- JSON.parse(localStorage.getItem(appConstants.LANGUAGE_CODE_VALUES)).forEach( + const raw = localStorage.getItem(appConstants.LANGUAGE_CODE_VALUES); + let parsed: any[] = []; + try { parsed = raw ? JSON.parse(raw) : []; } catch { parsed = []; } + parsed.forEach(pre-registration-ui/src/app/feature/demographic/demographic/demographic.component.ts (2)
331-339: Guard JSON.parse on session datalocalStorage corruption will crash initialization.
- this.dataCaptureLanguages = JSON.parse( - localStorage.getItem(appConstants.DATA_CAPTURE_LANGUAGES) - ); - this.dataCaptureLanguagesLabels = JSON.parse( - localStorage.getItem(appConstants.DATA_CAPTURE_LANGUAGE_LABELS) - ); + try { + this.dataCaptureLanguages = JSON.parse(localStorage.getItem(appConstants.DATA_CAPTURE_LANGUAGES) || '[]'); + } catch { this.dataCaptureLanguages = []; } + try { + this.dataCaptureLanguagesLabels = JSON.parse(localStorage.getItem(appConstants.DATA_CAPTURE_LANGUAGE_LABELS) || '[]'); + } catch { this.dataCaptureLanguagesLabels = []; }
300-323: NOSONAR comments are fine; consider consolidating repeated replaceNot blocking, but you can extract a helper to DRY the repeated global replace blocks.
pre-registration-ui/src/app/feature/file-upload/file-upload/file-upload.component.ts (1)
130-141: Add rejection handling to async initCatch and log failures to avoid silent init errors.
- // async part moved to helper; fire and forget - void this.initAsync(); + // async part moved to helper + this.initAsync().catch(err => { + console.error('FileUpload init failed:', err); + this.dataLoaded = true; // optional: unblock UI + });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
pre-registration-ui/src/app/auth/login/login.component.ts(5 hunks)pre-registration-ui/src/app/feature/booking/center-selection/center-selection.component.ts(1 hunks)pre-registration-ui/src/app/feature/booking/time-selection/time-selection.component.ts(2 hunks)pre-registration-ui/src/app/feature/dashboard/dashboard/dashboard.component.ts(2 hunks)pre-registration-ui/src/app/feature/demographic/demographic/demographic.component.ts(4 hunks)pre-registration-ui/src/app/feature/file-upload/file-upload/file-upload.component.ts(3 hunks)pre-registration-ui/src/app/feature/summary/acknowledgement/acknowledgement.component.ts(2 hunks)pre-registration-ui/src/app/feature/summary/preview/preview.component.ts(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- pre-registration-ui/src/app/feature/booking/center-selection/center-selection.component.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: pre-registration-ui / build-dockers
🔇 Additional comments (6)
pre-registration-ui/src/app/feature/demographic/demographic/demographic.component.ts (1)
190-195: Good: ngOnInit now logs and disables form on init failureThe catch handler prevents silent failures and sets a safe state.
pre-registration-ui/src/app/feature/file-upload/file-upload/file-upload.component.ts (1)
950-963: LGTM: extension regex is now anchored and guardedThe new construction prevents substring matches and handles empty configuration safely.
pre-registration-ui/src/app/feature/summary/acknowledgement/acknowledgement.component.ts (2)
62-75: Good: guarded parse and error‑handled initThe try/catch around JSON.parse and the .catch on initAsync improve resilience.
85-91: Good: track language-file subscription for cleanupSubscription is now pushed for ngOnDestroy.
pre-registration-ui/src/app/auth/login/login.component.ts (2)
76-87: Good: guarded redirect and centralized async init with catchThe added langCode guard and error handling reduce navigation and init edge cases.
111-138: Config loading awaits first emission; OTP reload logic now consistentUse of pipe(first()).toPromise() ensures config is set before proceeding; NaN checks modernized. Looks good.
…ssues in multiple components (mosip#224) Signed-off-by: Youssef MAHTAT <[email protected]>
…ssues in multiple components (mosip#224) Signed-off-by: Youssef MAHTAT <[email protected]>
…ultiple components (#225) * MOSIP-25201 - Fix accessibility and SonarQube reliability issues in multiple components (#198) Signed-off-by: Youssef MAHTAT <[email protected]> * MOSIP-25201 - Fix (round 2) accessibility and SonarQube reliability issues in multiple components (#209) Signed-off-by: Youssef MAHTAT <[email protected]> * MOSIP-25201 - Fix (round 3) accessibility and SonarQube reliability issues in multiple components (#224) Signed-off-by: Youssef MAHTAT <[email protected]> * MOSIP-25201 - Fix (round 4) remove NOSONAR comments (#226) Signed-off-by: Youssef MAHTAT <[email protected]> --------- Signed-off-by: Youssef MAHTAT <[email protected]>
2 participants
Summary by CodeRabbit
Refactor
Bug Fixes
Style