Release 3.6.1#44
Merged
Merged
Conversation
Contributor
📝 WalkthroughWalkthroughThe PR introduces Elasticsearch search support across multiple registrar components by adding an Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant SearchComponent
participant RegistrarService
participant ESBackend as Elasticsearch
participant LegacyBackend as Legacy API
User->>SearchComponent: Enter search term
SearchComponent->>SearchComponent: Debounce (500ms)
alt isEnableES enabled
SearchComponent->>RegistrarService: identityQuickSearchES(term)
RegistrarService->>ESBackend: POST to elasticSearchUrl
ESBackend-->>RegistrarService: Results array
RegistrarService-->>SearchComponent: Response
SearchComponent->>SearchComponent: searchRestructES() transform
SearchComponent->>SearchComponent: handleESSearchResponse()
else Legacy mode
SearchComponent->>RegistrarService: identityQuickSearch(reqObj)
RegistrarService->>LegacyBackend: POST to legacy endpoint
LegacyBackend-->>RegistrarService: Results
RegistrarService-->>SearchComponent: Response
SearchComponent->>SearchComponent: Process results
end
SearchComponent->>SearchComponent: Update worklist/table
SearchComponent-->>User: Display results
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
vanitha1822
approved these changes
Apr 24, 2026
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/registrar/registration/consent-form/consent-form.component.html (1)
14-24:⚠️ Potential issue | 🟡 MinorInconsistent localization: hardcoded Hindi paragraph remains while English moved to localization keys.
The English consent paragraph now binds to
currentLanguageSet?.consentFormText(line 22), but the Hindi paragraph on lines 15–18 is still hardcoded. This creates two problems:
- Users will always see the Hindi text regardless of the selected language.
- The
consentFormTextandtc.acceptkeys are not defined in this repository. If missing from the parent application's locale JSON, both the second paragraph and the Accept button will render blank (optional chaining silently swallows missing keys).Move the Hindi paragraph into a localization key so it switches with the selected language, or remove it if
consentFormTextis intended to contain the full consent body per locale. Additionally, ensureconsentFormTextandtc.acceptare properly defined in all locale files of the consuming application.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/registrar/registration/consent-form/consent-form.component.html` around lines 14 - 24, Replace the hardcoded Hindi paragraph with a localized string and ensure the component uses currentLanguageSet?.consentFormText (or a new key) consistently: remove the static Hindi <p> block and reference a localization key (e.g., consentFormText) for the full consent body so it switches per language; also confirm the consuming app defines consentFormText for each locale and adds tc.accept for the Accept button to avoid silent blanks when optional chaining is used.src/registrar/search/search.component.ts (1)
434-473:⚠️ Potential issue | 🔴 CriticalCritical:
benObject.dobbreaks non-ES patient revisits.
patientRevisitedreads onlybenObject.dob(lowercase) at lines 439, 458, and 468. However, the non-ESsearchRestructmethod (lines 371–373) reads onlyelement.dOB(camelCase), and this raw element is passed directly asbenObject(line 375). For non-ES results,benObject.dobisundefined, the first condition fails, and users always hit the "no age details" alert instead of the nurse handoff flow.The ES path is also at risk:
searchRestructESdefensively readselement.dob || element.dOB(line 237), suggesting the API may return either form, butpatientRevisitedonly accepts the lowercase variant.Accept both casings in
patientRevisitedto match the resilience shown insearchRestructES:Proposed fix
patientRevisited(benObject: any) { + const dob = benObject?.dob ?? benObject?.dOB; if ( benObject && benObject.m_gender && benObject.m_gender.genderName && - benObject.dob + dob ) { const action = false; // ... - } else if (!benObject.m_gender.genderName && !benObject.dob) { + } else if (!benObject.m_gender.genderName && !dob) { // ... - } else if (!benObject.dob) { + } else if (!dob) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/registrar/search/search.component.ts` around lines 434 - 473, The patientRevisited handler only checks benObject.dob (lowercase) causing non-ES results (which use element.dOB) to be treated as missing age; update patientRevisited to accept both casings (check benObject.dob || benObject.dOB wherever dob is read/validated and when deciding the alert branches) and use the resolved value when building benObject passed to sendToNurseWindow; mirror the defensive pattern used in searchRestructES so both camelCase dOB and lowercase dob are handled consistently and alerts/fire sendToNurseWindow appropriately.
🧹 Nitpick comments (5)
src/registrar/family-tagging/create-family-tagging/create-family-tagging.component.html (1)
117-117: Move dialog-action layout styles out of the template.Line 117 works functionally, but keeping flex/gap rules inline makes future theming and responsive overrides harder. Prefer a dedicated CSS class in the component stylesheet.
Suggested cleanup
- <mat-dialog-actions class="padding15 margin15 pull-right" style="display: flex;width:100%;justify-content: flex-end;gap:12px"> + <mat-dialog-actions class="padding15 margin15 pull-right family-tagging-dialog-actions">/* create-family-tagging.component.scss */ .family-tagging-dialog-actions { display: flex; width: 100%; justify-content: flex-end; gap: 12px; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/registrar/family-tagging/create-family-tagging/create-family-tagging.component.html` at line 117, The template currently applies layout styles inline on the mat-dialog-actions element; move those rules into the component stylesheet by adding a dedicated CSS class (e.g., .family-tagging-dialog-actions) in create-family-tagging.component.scss with display:flex; width:100%; justify-content:flex-end; gap:12px, then replace the inline style on the mat-dialog-actions element in create-family-tagging.component.html with that new class (keep existing classes like padding15 margin15 pull-right).src/registrar/services/registrar.service.ts (1)
124-147: Minor: formatting inconsistency on new ES methods.Line 125 is indented with 3 spaces while siblings use 4. Worth tidying while the file is open to keep the service body uniform.
✂️ Formatting tweak
identityQuickSearchES(searchTerm: any) { - return this.http.post(environment.elasticSearchUrl, searchTerm); + return this.http.post(environment.elasticSearchUrl, searchTerm); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/registrar/services/registrar.service.ts` around lines 124 - 147, The new method advanceSearchIdentityES (and nearby identityQuickSearchES) has inconsistent indentation (3 spaces) compared to other methods (4 spaces); update the whitespace so the method signatures and bodies use the same 4-space indentation as its siblings in registrar.service.ts to keep formatting uniform across the service.src/registrar/family-tagging/family-tagging-details/family-tagging-details.component.ts (1)
381-437: Refactor: consolidate ES/non-ES branches and scope the request object correctly.
benReqObjis constructed unconditionally but only consumed in the non-ES branch. The two branches also differ only in the HTTP call and thefamilyID/familyIdkey — both do the same post-processing. Extracting a small helper (or using RxJSmap) collapses the duplication and makes the field-name difference the only visible divergence.♻️ Suggested shape
-getBeneficiaryDetailsAfterFamilyTag() { - const benReqObj = { - beneficiaryRegID: null, - beneficiaryID: this.beneficiaryId, - phoneNo: null, - HealthID: null, - HealthIDNumber: null, - familyId: null, - identity: null, - }; - - if (this.isEnableES) { - // Use Elasticsearch when enabled - const esSearchReqObj = { search: this.beneficiaryId }; - this.registrarService.identityQuickSearchES(esSearchReqObj).subscribe( - (beneficiaryDetails: any) => { ... }, - (error: string) => { this.confirmationService.alert(error, 'error'); } - ); - } else { - this.registrarService.identityQuickSearch(benReqObj).subscribe( - (beneficiaryDetails: any) => { ... }, - (error: string) => { this.confirmationService.alert(error, 'error'); } - ); - } -} +getBeneficiaryDetailsAfterFamilyTag() { + const request$ = this.isEnableES + ? this.registrarService.identityQuickSearchES({ search: this.beneficiaryId }) + : this.registrarService.identityQuickSearch({ + beneficiaryRegID: null, + beneficiaryID: this.beneficiaryId, + phoneNo: null, + HealthID: null, + HealthIDNumber: null, + familyId: null, + identity: null, + }); + + request$.subscribe( + (res: any) => { + const row = res?.data?.length === 1 ? res.data[0] : null; + this.benFamilyId = row ? (row.familyID ?? row.familyId ?? null) : null; + if (this.benFamilyId) this.registrarService.getBenFamilyDetails(this.benFamilyId); + }, + (error: string) => this.confirmationService.alert(error, 'error'), + ); +}Also note the ES branch requires
statusCode === 200while the legacy branch does not. If the non-ES endpoint ever wraps the payload similarly, harmonize that check too.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/registrar/family-tagging/family-tagging-details/family-tagging-details.component.ts` around lines 381 - 437, The current getBeneficiaryDetailsAfterFamilyTag duplicates logic between the Elasticsearch and non-ES branches (benReqObj built but only used for identityQuickSearch, differing only by service call and the familyID vs familyId field name) and has inconsistent payload checks (ES checks statusCode === 200). Refactor by building the request only when needed, call either registrarService.identityQuickSearchES(esSearchReqObj) or registrarService.identityQuickSearch(benReqObj), then pipe/map the response to a single handler that normalizes the family id field (check both familyID and familyId) and applies the same statusCode/data existence checks before assigning benFamilyId and calling registrarService.getBenFamilyDetails(benFamilyId); keep error handling shared. Ensure variable names referenced: getBeneficiaryDetailsAfterFamilyTag, benReqObj, esSearchReqObj, identityQuickSearchES, identityQuickSearch, benFamilyId, registrarService.getBenFamilyDetails.src/registrar/search/search.component.ts (1)
130-166: Debounced search: validation alert fires per keystroke, and whitespace is tolerated here but not in the HTML pattern.Two small issues:
alphanumericPatternhere allows\s, while the HTMLpatternattribute on the input (search.component.htmlLine 12) is[a-zA-Z0-9]*(no whitespace). Users typing a space will pass the TS check but fail the HTML validity state, leading to inconsistent UX.- Because the validation lives inside
switchMap, any invalid character typed after the 500 ms debounce triggersconfirmationService.alert(...). If a user pastes or types several invalid characters, each distinct invalid term produces an alert. Consider validating inonSearchInputChangeand only pushing to the subject when valid, soswitchMapis reserved for the HTTP call.♻️ Suggested shape
- setupDebouncedSearch() { - this.searchSubscription = this.searchSubject$ - .pipe( - debounceTime(500), - distinctUntilChanged(), - switchMap((searchTerm: string) => { - const alphanumericPattern = /^[a-zA-Z0-9\s]*$/; - if (!alphanumericPattern.test(searchTerm)) { - this.confirmationService.alert('Please enter valid alphanumeric input', 'info'); - return []; - } - return this.registrarService.identityQuickSearchES({ search: searchTerm }); - }) - ) - .subscribe(...); - } + setupDebouncedSearch() { + this.searchSubscription = this.searchSubject$ + .pipe( + debounceTime(500), + distinctUntilChanged(), + switchMap((searchTerm: string) => + this.registrarService.identityQuickSearchES({ search: searchTerm }), + ), + ) + .subscribe(...); + } + + onSearchInputChange(searchTerm: string) { + const trimmed = searchTerm?.trim() || ''; + if (trimmed.length === 0) { this.resetWorklist(); return; } + if (trimmed.length < 3) return; + const alphanumericPattern = /^[a-zA-Z0-9]*$/; + if (!alphanumericPattern.test(trimmed)) { + this.confirmationService.alert('Please enter valid alphanumeric input', 'info'); + return; + } + this.searchSubject$.next(trimmed); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/registrar/search/search.component.ts` around lines 130 - 166, The current validation in setupDebouncedSearch (inside switchMap) allows whitespace and triggers confirmationService.alert per debounced emission; instead, remove validation from setupDebouncedSearch/switchMap so it only calls registrarService.identityQuickSearchES, and move input validation into onSearchInputChange: use the same HTML pattern ([a-zA-Z0-9]* — i.e., no \s) to validate trimmed input before calling this.searchSubject$.next(trimmed), only push when valid and length >= 3, show the confirmationService.alert from onSearchInputChange for invalid input once, and keep resetWorklist behavior when trimmed.length === 0.src/registrar/registration/location-information/location-information.component.html (1)
10-148: i18n consideration:titlecaseuses English casing rules.The Angular
TitleCasePipeapplies English word-boundary capitalization and does not account for locale-specific casing (e.g., languages without capitalization, agglutinative scripts, or proper nouns like "Tamil Nadu" vs "tamil nadu"). Since this component renders master data that may be localized, titlecasing thefieldTitle,placeholder, andoptionvalues can produce incorrect output for non-English locales and may double-transform already-cased master data.Consider relying on properly cased master data or gating this transform behind a locale check.
Also note: the dropdown's autocomplete input placeholder at Line 130 is the only one left untitled — if the intent is full consistency, apply the pipe there too or revert it across the board.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/registrar/registration/location-information/location-information.component.html` around lines 10 - 148, The template uses the Angular TitleCasePipe on master data (occurrences: "{{ item.fieldTitle | titlecase }}", "{{ item.placeholder | titlecase }}", and "{{ option | titlecase }}") which enforces English casing rules; remove the pipe and instead either use master data already cased correctly or apply casing conditionally in the component based on the active locale (inject LOCALE_ID and expose helper methods like formatLabel(fieldTitle), formatPlaceholder(placeholder), formatOption(option) that only apply titlecasing for locales where it’s appropriate). Also make the autocomplete input placeholder ([placeholder]="item.placeholder") consistent with other placeholders by routing it through the same helper/decision. Ensure you update bindings in the template to call those helpers (or remove the pipe) rather than using | titlecase directly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/registrar/beneficiary-details/beneficiary-details.component.ts`:
- Around line 140-229: getBenFamilyDetails now starts the image fetch in
parallel causing a race where fetchBeneficiaryImage may set
this.beneficiary.benImage before this.beneficiary exists; fix by sequencing and
hardening: remove the unconditional fetchBeneficiaryImage(benFlowID) call from
getBenFamilyDetails and instead call fetchBeneficiaryImage(benFlowID) from
inside handleSearchResponse only after confirming a single result (where
this.beneficiary is set); additionally, in fetchBeneficiaryImage ensure the
assignment is guarded (only set this.beneficiary.benImage if this.beneficiary is
defined) so the method is safe if accidentally invoked elsewhere.
In
`@src/registrar/registration/personal-information/personal-information.component.ts`:
- Around line 339-428: getParentDetails currently logs full API responses and
derived PII to the browser console (see console.log calls around
registrarService.identityQuickSearchES and registrarService.identityQuickSearch
branches), which must be removed or guarded; update getParentDetails to stop
printing raw responses and identifiers by either removing those console.log
calls or wrapping them behind a dev-only check (e.g., if (isDev) ...) or using a
privacy-safe logger that redacts/masks phone numbers, beneficiaryRegID and
parentRegID before logging, and ensure any error display via
confirmationService.alert does not expose sensitive fields from the error
payload.
In `@src/registrar/search/search.component.html`:
- Around line 12-18: The input's pattern is hard-coded to alphanumeric and
breaks non-ES HealthID/ABHA input; update the template to bind the pattern
attribute based on isEnableES so validation matches the active mode: when
isEnableES is true keep the restrictive alphanumeric pattern, when false use the
legacy HealthID pattern that allows '@', '.' and '-' (the TS-intended regex used
previously). Locate the input with [(ngModel)]="quicksearchTerm" /
(ngModelChange)="onSearchInputChange($event)" /
(keyup.enter)="onSearchButtonClick(quicksearchTerm)" and replace the static
pattern attribute with a conditional bound pattern expression using isEnableES.
In `@src/registrar/search/search.component.ts`:
- Line 113: The component defines searchPattern (assigned using this.isEnableES)
with an incorrect regex literal string (includes leading/trailing slashes and a
semicolon) and it is unused because the template hardcodes
pattern="[a-zA-Z0-9]*"; either remove the unused searchPattern property entirely
(remove its declaration and the assignment in the constructor) or fix and bind
it: set searchPattern to the correct regex string value without
delimiters/semicolons (e.g., "^[a-zA-Z0-9]*$" or the alternative ES pattern) and
update the template to use the bound value (bind pattern to searchPattern) so
the component property is actually used; update references to isEnableES and the
searchPattern property accordingly.
---
Outside diff comments:
In `@src/registrar/registration/consent-form/consent-form.component.html`:
- Around line 14-24: Replace the hardcoded Hindi paragraph with a localized
string and ensure the component uses currentLanguageSet?.consentFormText (or a
new key) consistently: remove the static Hindi <p> block and reference a
localization key (e.g., consentFormText) for the full consent body so it
switches per language; also confirm the consuming app defines consentFormText
for each locale and adds tc.accept for the Accept button to avoid silent blanks
when optional chaining is used.
In `@src/registrar/search/search.component.ts`:
- Around line 434-473: The patientRevisited handler only checks benObject.dob
(lowercase) causing non-ES results (which use element.dOB) to be treated as
missing age; update patientRevisited to accept both casings (check benObject.dob
|| benObject.dOB wherever dob is read/validated and when deciding the alert
branches) and use the resolved value when building benObject passed to
sendToNurseWindow; mirror the defensive pattern used in searchRestructES so both
camelCase dOB and lowercase dob are handled consistently and alerts/fire
sendToNurseWindow appropriately.
---
Nitpick comments:
In
`@src/registrar/family-tagging/create-family-tagging/create-family-tagging.component.html`:
- Line 117: The template currently applies layout styles inline on the
mat-dialog-actions element; move those rules into the component stylesheet by
adding a dedicated CSS class (e.g., .family-tagging-dialog-actions) in
create-family-tagging.component.scss with display:flex; width:100%;
justify-content:flex-end; gap:12px, then replace the inline style on the
mat-dialog-actions element in create-family-tagging.component.html with that new
class (keep existing classes like padding15 margin15 pull-right).
In
`@src/registrar/family-tagging/family-tagging-details/family-tagging-details.component.ts`:
- Around line 381-437: The current getBeneficiaryDetailsAfterFamilyTag
duplicates logic between the Elasticsearch and non-ES branches (benReqObj built
but only used for identityQuickSearch, differing only by service call and the
familyID vs familyId field name) and has inconsistent payload checks (ES checks
statusCode === 200). Refactor by building the request only when needed, call
either registrarService.identityQuickSearchES(esSearchReqObj) or
registrarService.identityQuickSearch(benReqObj), then pipe/map the response to a
single handler that normalizes the family id field (check both familyID and
familyId) and applies the same statusCode/data existence checks before assigning
benFamilyId and calling registrarService.getBenFamilyDetails(benFamilyId); keep
error handling shared. Ensure variable names referenced:
getBeneficiaryDetailsAfterFamilyTag, benReqObj, esSearchReqObj,
identityQuickSearchES, identityQuickSearch, benFamilyId,
registrarService.getBenFamilyDetails.
In
`@src/registrar/registration/location-information/location-information.component.html`:
- Around line 10-148: The template uses the Angular TitleCasePipe on master data
(occurrences: "{{ item.fieldTitle | titlecase }}", "{{ item.placeholder |
titlecase }}", and "{{ option | titlecase }}") which enforces English casing
rules; remove the pipe and instead either use master data already cased
correctly or apply casing conditionally in the component based on the active
locale (inject LOCALE_ID and expose helper methods like formatLabel(fieldTitle),
formatPlaceholder(placeholder), formatOption(option) that only apply titlecasing
for locales where it’s appropriate). Also make the autocomplete input
placeholder ([placeholder]="item.placeholder") consistent with other
placeholders by routing it through the same helper/decision. Ensure you update
bindings in the template to call those helpers (or remove the pipe) rather than
using | titlecase directly.
In `@src/registrar/search/search.component.ts`:
- Around line 130-166: The current validation in setupDebouncedSearch (inside
switchMap) allows whitespace and triggers confirmationService.alert per
debounced emission; instead, remove validation from
setupDebouncedSearch/switchMap so it only calls
registrarService.identityQuickSearchES, and move input validation into
onSearchInputChange: use the same HTML pattern ([a-zA-Z0-9]* — i.e., no \s) to
validate trimmed input before calling this.searchSubject$.next(trimmed), only
push when valid and length >= 3, show the confirmationService.alert from
onSearchInputChange for invalid input once, and keep resetWorklist behavior when
trimmed.length === 0.
In `@src/registrar/services/registrar.service.ts`:
- Around line 124-147: The new method advanceSearchIdentityES (and nearby
identityQuickSearchES) has inconsistent indentation (3 spaces) compared to other
methods (4 spaces); update the whitespace so the method signatures and bodies
use the same 4-space indentation as its siblings in registrar.service.ts to keep
formatting uniform across the service.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 49b2f82c-2baa-4469-8a1b-c036bcde3d1c
📒 Files selected for processing (11)
src/registrar/beneficiary-details/beneficiary-details.component.tssrc/registrar/family-tagging/create-family-tagging/create-family-tagging.component.htmlsrc/registrar/family-tagging/family-tagging-details/family-tagging-details.component.tssrc/registrar/registration/consent-form/consent-form.component.htmlsrc/registrar/registration/location-information/location-information.component.htmlsrc/registrar/registration/personal-information/personal-information.component.tssrc/registrar/registration/registration.component.htmlsrc/registrar/search-dialog/search-dialog.component.htmlsrc/registrar/search/search.component.htmlsrc/registrar/search/search.component.tssrc/registrar/services/registrar.service.ts
Comment on lines
140
to
+229
| getBenFamilyDetails() { | ||
| this.route.params.subscribe((param) => { | ||
| const reqObj = { | ||
| beneficiaryRegID: null, | ||
| beneficiaryName: null, | ||
| beneficiaryID: param['beneficiaryId'], | ||
| phoneNo: null, | ||
| HealthID: null, | ||
| HealthIDNumber: null, | ||
| familyId: null, | ||
| identity: null, | ||
| }; | ||
| this.registrarService | ||
| .identityQuickSearch(reqObj) | ||
| .subscribe((res: any) => { | ||
| if (res && res.data.length === 1) { | ||
| this.beneficiary = res.data[0]; | ||
| this.benFamilyId = res.data[0].familyId; | ||
| this.beneficiaryName = | ||
| this.beneficiary.firstName + | ||
| (this.beneficiary.lastName !== undefined | ||
| ? ' ' + this.beneficiary.lastName | ||
| : ''); | ||
| this.regDate = moment | ||
| .utc(this.beneficiary.createdDate) | ||
| .format('DD-MM-YYYY hh:mm A'); | ||
| } | ||
| }); | ||
| const benFlowID: any = param['beneficiaryRegID']; | ||
| this.beneficiaryDetailsService | ||
| .getBeneficiaryImage(benFlowID) | ||
| .subscribe((data: any) => { | ||
| if (data && data.benImage) { | ||
| const beneficiaryId = param['beneficiaryId']; | ||
| const benFlowID = param['beneficiaryRegID']; | ||
|
|
||
| // Fetch beneficiary details based on search method | ||
| const searchObservable = this.isEnableES | ||
| ? this.searchWithElasticsearch(beneficiaryId) | ||
| : this.searchTraditional(beneficiaryId); | ||
|
|
||
| searchObservable.subscribe({ | ||
| next: (res: any) => this.handleSearchResponse(res), | ||
| error: (err: any) => this.handleSearchError(err), | ||
| }); | ||
|
|
||
| // Fetch beneficiary image independently | ||
| this.fetchBeneficiaryImage(benFlowID); | ||
| }); | ||
| } | ||
|
|
||
| private searchWithElasticsearch(searchTerm: string) { | ||
| return this.registrarService.identityQuickSearchES({ search: searchTerm }); | ||
| } | ||
|
|
||
| private searchTraditional(beneficiaryId: string) { | ||
| const reqObj = { | ||
| beneficiaryRegID: null, | ||
| beneficiaryName: null, | ||
| beneficiaryID: beneficiaryId, | ||
| phoneNo: null, | ||
| HealthID: null, | ||
| HealthIDNumber: null, | ||
| familyId: null, | ||
| identity: null, | ||
| }; | ||
| return this.registrarService.identityQuickSearch(reqObj); | ||
| } | ||
|
|
||
| private handleSearchResponse(res: any) { | ||
| if (!res?.data || res.data.length === 0) { | ||
| this.showAlert( | ||
| this.current_language_set?.alerts?.info?.beneficiarynotfound || | ||
| 'Beneficiary not found', | ||
| 'info' | ||
| ); | ||
| return; | ||
| } | ||
|
|
||
| if (res.data.length > 1) { | ||
| this.showAlert( | ||
| 'Multiple beneficiaries found with this ID. Please use advanced search.', | ||
| 'info' | ||
| ); | ||
| return; | ||
| } | ||
|
|
||
| // Single result found | ||
| this.beneficiary = res.data[0]; | ||
| this.benFamilyId = res.data[0].familyID || res.data[0].familyId; | ||
| this.beneficiaryName = this.formatBeneficiaryName(this.beneficiary); | ||
| this.regDate = moment | ||
| .utc(this.beneficiary.createdDate) | ||
| .format('DD-MM-YYYY hh:mm A'); | ||
| } | ||
|
|
||
| private formatBeneficiaryName(beneficiary: any): string { | ||
| const { firstName, lastName } = beneficiary; | ||
| return lastName?.trim() ? `${firstName} ${lastName}` : firstName; | ||
| } | ||
|
|
||
| private handleSearchError(err: any) { | ||
| console.error('Error fetching beneficiary details:', err); | ||
| this.showAlert('Error fetching beneficiary details', 'error'); | ||
| } | ||
|
|
||
| private fetchBeneficiaryImage(benFlowID: string) { | ||
| this.beneficiaryDetailsService | ||
| .getBeneficiaryImage(benFlowID) | ||
| .subscribe({ | ||
| next: (data: any) => { | ||
| if (data?.benImage) { | ||
| this.beneficiary.benImage = data.benImage; | ||
| } | ||
| }); | ||
| }); | ||
| }, | ||
| error: (err: any) => { | ||
| console.error('Error fetching beneficiary image:', err); | ||
| // Optionally show error to user or handle silently | ||
| }, | ||
| }); | ||
| } |
Contributor
There was a problem hiding this comment.
Critical: race condition — this.beneficiary.benImage = ... can dereference undefined.
Previously the image fetch was chained off the search response, so this.beneficiary was guaranteed populated before this.beneficiary.benImage was assigned. After this refactor, fetchBeneficiaryImage(benFlowID) (Line 156) runs in parallel with the search (Line 150). Two problems:
- If the image response returns before the search,
this.beneficiaryis stillundefined, and Line 221 (this.beneficiary.benImage = data.benImage;) throws aTypeError. - Even after the search completes,
this.beneficiaryis only set on the "single match" path (Line 197). On "not found" and "multiple matches" (Lines 180 and 189),this.beneficiarystaysundefined, so if the image call succeeds it still crashes.
Either sequence the image fetch off the search result, or guard the assignment.
🛡️ Proposed fix (guarded assignment + sequence)
- searchObservable.subscribe({
- next: (res: any) => this.handleSearchResponse(res),
- error: (err: any) => this.handleSearchError(err),
- });
-
- // Fetch beneficiary image independently
- this.fetchBeneficiaryImage(benFlowID);
+ searchObservable.subscribe({
+ next: (res: any) => {
+ this.handleSearchResponse(res);
+ if (this.beneficiary) this.fetchBeneficiaryImage(benFlowID);
+ },
+ error: (err: any) => this.handleSearchError(err),
+ });And harden the setter as a belt-and-braces:
next: (data: any) => {
- if (data?.benImage) {
+ if (data?.benImage && this.beneficiary) {
this.beneficiary.benImage = data.benImage;
}
},📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| getBenFamilyDetails() { | |
| this.route.params.subscribe((param) => { | |
| const reqObj = { | |
| beneficiaryRegID: null, | |
| beneficiaryName: null, | |
| beneficiaryID: param['beneficiaryId'], | |
| phoneNo: null, | |
| HealthID: null, | |
| HealthIDNumber: null, | |
| familyId: null, | |
| identity: null, | |
| }; | |
| this.registrarService | |
| .identityQuickSearch(reqObj) | |
| .subscribe((res: any) => { | |
| if (res && res.data.length === 1) { | |
| this.beneficiary = res.data[0]; | |
| this.benFamilyId = res.data[0].familyId; | |
| this.beneficiaryName = | |
| this.beneficiary.firstName + | |
| (this.beneficiary.lastName !== undefined | |
| ? ' ' + this.beneficiary.lastName | |
| : ''); | |
| this.regDate = moment | |
| .utc(this.beneficiary.createdDate) | |
| .format('DD-MM-YYYY hh:mm A'); | |
| } | |
| }); | |
| const benFlowID: any = param['beneficiaryRegID']; | |
| this.beneficiaryDetailsService | |
| .getBeneficiaryImage(benFlowID) | |
| .subscribe((data: any) => { | |
| if (data && data.benImage) { | |
| const beneficiaryId = param['beneficiaryId']; | |
| const benFlowID = param['beneficiaryRegID']; | |
| // Fetch beneficiary details based on search method | |
| const searchObservable = this.isEnableES | |
| ? this.searchWithElasticsearch(beneficiaryId) | |
| : this.searchTraditional(beneficiaryId); | |
| searchObservable.subscribe({ | |
| next: (res: any) => this.handleSearchResponse(res), | |
| error: (err: any) => this.handleSearchError(err), | |
| }); | |
| // Fetch beneficiary image independently | |
| this.fetchBeneficiaryImage(benFlowID); | |
| }); | |
| } | |
| private searchWithElasticsearch(searchTerm: string) { | |
| return this.registrarService.identityQuickSearchES({ search: searchTerm }); | |
| } | |
| private searchTraditional(beneficiaryId: string) { | |
| const reqObj = { | |
| beneficiaryRegID: null, | |
| beneficiaryName: null, | |
| beneficiaryID: beneficiaryId, | |
| phoneNo: null, | |
| HealthID: null, | |
| HealthIDNumber: null, | |
| familyId: null, | |
| identity: null, | |
| }; | |
| return this.registrarService.identityQuickSearch(reqObj); | |
| } | |
| private handleSearchResponse(res: any) { | |
| if (!res?.data || res.data.length === 0) { | |
| this.showAlert( | |
| this.current_language_set?.alerts?.info?.beneficiarynotfound || | |
| 'Beneficiary not found', | |
| 'info' | |
| ); | |
| return; | |
| } | |
| if (res.data.length > 1) { | |
| this.showAlert( | |
| 'Multiple beneficiaries found with this ID. Please use advanced search.', | |
| 'info' | |
| ); | |
| return; | |
| } | |
| // Single result found | |
| this.beneficiary = res.data[0]; | |
| this.benFamilyId = res.data[0].familyID || res.data[0].familyId; | |
| this.beneficiaryName = this.formatBeneficiaryName(this.beneficiary); | |
| this.regDate = moment | |
| .utc(this.beneficiary.createdDate) | |
| .format('DD-MM-YYYY hh:mm A'); | |
| } | |
| private formatBeneficiaryName(beneficiary: any): string { | |
| const { firstName, lastName } = beneficiary; | |
| return lastName?.trim() ? `${firstName} ${lastName}` : firstName; | |
| } | |
| private handleSearchError(err: any) { | |
| console.error('Error fetching beneficiary details:', err); | |
| this.showAlert('Error fetching beneficiary details', 'error'); | |
| } | |
| private fetchBeneficiaryImage(benFlowID: string) { | |
| this.beneficiaryDetailsService | |
| .getBeneficiaryImage(benFlowID) | |
| .subscribe({ | |
| next: (data: any) => { | |
| if (data?.benImage) { | |
| this.beneficiary.benImage = data.benImage; | |
| } | |
| }); | |
| }); | |
| }, | |
| error: (err: any) => { | |
| console.error('Error fetching beneficiary image:', err); | |
| // Optionally show error to user or handle silently | |
| }, | |
| }); | |
| } | |
| getBenFamilyDetails() { | |
| this.route.params.subscribe((param) => { | |
| const beneficiaryId = param['beneficiaryId']; | |
| const benFlowID = param['beneficiaryRegID']; | |
| // Fetch beneficiary details based on search method | |
| const searchObservable = this.isEnableES | |
| ? this.searchWithElasticsearch(beneficiaryId) | |
| : this.searchTraditional(beneficiaryId); | |
| searchObservable.subscribe({ | |
| next: (res: any) => { | |
| this.handleSearchResponse(res); | |
| if (this.beneficiary) this.fetchBeneficiaryImage(benFlowID); | |
| }, | |
| error: (err: any) => this.handleSearchError(err), | |
| }); | |
| }); | |
| } | |
| private searchWithElasticsearch(searchTerm: string) { | |
| return this.registrarService.identityQuickSearchES({ search: searchTerm }); | |
| } | |
| private searchTraditional(beneficiaryId: string) { | |
| const reqObj = { | |
| beneficiaryRegID: null, | |
| beneficiaryName: null, | |
| beneficiaryID: beneficiaryId, | |
| phoneNo: null, | |
| HealthID: null, | |
| HealthIDNumber: null, | |
| familyId: null, | |
| identity: null, | |
| }; | |
| return this.registrarService.identityQuickSearch(reqObj); | |
| } | |
| private handleSearchResponse(res: any) { | |
| if (!res?.data || res.data.length === 0) { | |
| this.showAlert( | |
| this.current_language_set?.alerts?.info?.beneficiarynotfound || | |
| 'Beneficiary not found', | |
| 'info' | |
| ); | |
| return; | |
| } | |
| if (res.data.length > 1) { | |
| this.showAlert( | |
| 'Multiple beneficiaries found with this ID. Please use advanced search.', | |
| 'info' | |
| ); | |
| return; | |
| } | |
| // Single result found | |
| this.beneficiary = res.data[0]; | |
| this.benFamilyId = res.data[0].familyID || res.data[0].familyId; | |
| this.beneficiaryName = this.formatBeneficiaryName(this.beneficiary); | |
| this.regDate = moment | |
| .utc(this.beneficiary.createdDate) | |
| .format('DD-MM-YYYY hh:mm A'); | |
| } | |
| private formatBeneficiaryName(beneficiary: any): string { | |
| const { firstName, lastName } = beneficiary; | |
| return lastName?.trim() ? `${firstName} ${lastName}` : firstName; | |
| } | |
| private handleSearchError(err: any) { | |
| console.error('Error fetching beneficiary details:', err); | |
| this.showAlert('Error fetching beneficiary details', 'error'); | |
| } | |
| private fetchBeneficiaryImage(benFlowID: string) { | |
| this.beneficiaryDetailsService | |
| .getBeneficiaryImage(benFlowID) | |
| .subscribe({ | |
| next: (data: any) => { | |
| if (data?.benImage && this.beneficiary) { | |
| this.beneficiary.benImage = data.benImage; | |
| } | |
| }, | |
| error: (err: any) => { | |
| console.error('Error fetching beneficiary image:', err); | |
| // Optionally show error to user or handle silently | |
| }, | |
| }); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/registrar/beneficiary-details/beneficiary-details.component.ts` around
lines 140 - 229, getBenFamilyDetails now starts the image fetch in parallel
causing a race where fetchBeneficiaryImage may set this.beneficiary.benImage
before this.beneficiary exists; fix by sequencing and hardening: remove the
unconditional fetchBeneficiaryImage(benFlowID) call from getBenFamilyDetails and
instead call fetchBeneficiaryImage(benFlowID) from inside handleSearchResponse
only after confirming a single result (where this.beneficiary is set);
additionally, in fetchBeneficiaryImage ensure the assignment is guarded (only
set this.beneficiary.benImage if this.beneficiary is defined) so the method is
safe if accidentally invoked elsewhere.
Comment on lines
+339
to
+428
| getParentDetails() { | ||
| const searchTerm = this.personalInfoFormGroup.value.phoneNo; | ||
| const searchObject = { | ||
| beneficiaryRegID: null, | ||
| beneficiaryID: null, | ||
| phoneNo: null, | ||
| }; | ||
|
|
||
| if (searchTerm && searchTerm.trim().length === 10) { | ||
| searchObject['phoneNo'] = searchTerm; | ||
| this.registrarService.identityQuickSearch(searchObject).subscribe( | ||
| (beneficiaryList: any) => { | ||
| if ( | ||
| beneficiaryList && | ||
| beneficiaryList.length > 0 && | ||
| beneficiaryList[0].benPhoneMaps.length > 0 | ||
| ) { | ||
| console.log( | ||
| 'ta d ad a d a', | ||
| JSON.stringify(beneficiaryList, null, 4) | ||
| ); | ||
| this.personalInfoFormGroup.patchValue({ | ||
| parentRegID: beneficiaryList[0].benPhoneMaps[0].parentBenRegID, | ||
| parentRelation: 11, | ||
| }); | ||
| console.log(this.personalInfoFormGroup.value.parentRegID); | ||
| } else { | ||
| if (this.isEnableES) { | ||
| // Use Elasticsearch search | ||
| this.registrarService.identityQuickSearchES({ search: searchTerm }).subscribe( | ||
| (response: any) => { | ||
| if ( | ||
| response && | ||
| response.data && | ||
| response.data.length > 0 && | ||
| response.data[0].benPhoneMaps && | ||
| response.data[0].benPhoneMaps.length > 0 | ||
| ) { | ||
| console.log('ES Parent search result:', JSON.stringify(response, null, 4)); | ||
| this.personalInfoFormGroup.patchValue({ | ||
| parentRegID: response.data[0].benPhoneMaps[0].parentBenRegID, | ||
| parentRelation: 11, | ||
| }); | ||
| console.log('Parent Reg ID (ES):', this.personalInfoFormGroup.value.parentRegID); | ||
| } else { | ||
| this.personalInfoFormGroup.patchValue({ | ||
| parentRegID: null, | ||
| parentRelation: 1, | ||
| }); | ||
| console.log('No parent found (ES):', this.personalInfoFormGroup.value.parentRegID); | ||
|
|
||
| if (this.patientRevisit) { | ||
| this.personalInfoFormGroup.patchValue({ | ||
| parentRegID: this.personalInfoFormGroup.value.beneficiaryRegID, | ||
| }); | ||
| console.log('Patient revisit parent (ES):', this.personalInfoFormGroup.value.parentRegID); | ||
| } | ||
| } | ||
| }, | ||
| error => { | ||
| this.confirmationService.alert(error, 'error'); | ||
| this.personalInfoFormGroup.patchValue({ | ||
| parentRegID: null, | ||
| parentRelation: 1, | ||
| phoneNo: null, | ||
| }); | ||
| console.log(this.personalInfoFormGroup.value.parentRegID); | ||
|
|
||
| if (this.patientRevisit) { | ||
| } | ||
| ); | ||
| } else { | ||
| const searchObject = { | ||
| beneficiaryRegID: null, | ||
| beneficiaryID: null, | ||
| phoneNo: searchTerm, | ||
| }; | ||
|
|
||
| this.registrarService.identityQuickSearch(searchObject).subscribe( | ||
| (beneficiaryList: any) => { | ||
| if ( | ||
| beneficiaryList && | ||
| beneficiaryList.length > 0 && | ||
| beneficiaryList[0].benPhoneMaps.length > 0 | ||
| ) { | ||
| console.log('Traditional search result:', JSON.stringify(beneficiaryList, null, 4)); | ||
| this.personalInfoFormGroup.patchValue({ | ||
| parentRegID: beneficiaryList[0].benPhoneMaps[0].parentBenRegID, | ||
| parentRelation: 11, | ||
| }); | ||
| console.log('Parent Reg ID:', this.personalInfoFormGroup.value.parentRegID); | ||
| } else { | ||
| this.personalInfoFormGroup.patchValue({ | ||
| parentRegID: this.personalInfoFormGroup.value.beneficiaryRegID, | ||
| parentRegID: null, | ||
| parentRelation: 1, | ||
| }); | ||
| console.log(this.personalInfoFormGroup.value.parentRegID); | ||
| console.log('No parent found:', this.personalInfoFormGroup.value.parentRegID); | ||
|
|
||
| if (this.patientRevisit) { | ||
| this.personalInfoFormGroup.patchValue({ | ||
| parentRegID: this.personalInfoFormGroup.value.beneficiaryRegID, | ||
| }); | ||
| console.log('Patient revisit parent:', this.personalInfoFormGroup.value.parentRegID); | ||
| } | ||
| } | ||
| }, | ||
| error => { | ||
| this.confirmationService.alert(error, 'error'); | ||
| this.personalInfoFormGroup.patchValue({ | ||
| parentRegID: null, | ||
| parentRelation: 1, | ||
| phoneNo: null, | ||
| }); | ||
| } | ||
| }, | ||
| error => { | ||
| this.confirmationService.alert(error, 'error'); | ||
| this.personalInfoFormGroup.patchValue({ | ||
| parentRegID: null, | ||
| parentRelation: 1, | ||
| phoneNo: null, | ||
| }); | ||
| } | ||
| ); | ||
| ); | ||
| } |
Contributor
There was a problem hiding this comment.
Privacy: search response with PII is being logged to the browser console.
Multiple console.log calls here stringify the entire API response (Lines 354, 398) — which includes phone numbers, beneficiary IDs, parent registration IDs, and names — and additionally log derived identifiers (Lines 359, 365, 371, 403, 409, 415). In production these appear in browser dev tools and in any telemetry that captures console output, which is typically unacceptable for PHI/PII under GDPR/HIPAA-like regimes.
Recommend either removing these logs entirely or gating them behind a dev-only guard before release.
🔒 Minimal cleanup
- console.log('ES Parent search result:', JSON.stringify(response, null, 4));
this.personalInfoFormGroup.patchValue({
parentRegID: response.data[0].benPhoneMaps[0].parentBenRegID,
parentRelation: 11,
});
- console.log('Parent Reg ID (ES):', this.personalInfoFormGroup.value.parentRegID);(apply similarly to the other five log sites in both branches)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/registrar/registration/personal-information/personal-information.component.ts`
around lines 339 - 428, getParentDetails currently logs full API responses and
derived PII to the browser console (see console.log calls around
registrarService.identityQuickSearchES and registrarService.identityQuickSearch
branches), which must be removed or guarded; update getParentDetails to stop
printing raw responses and identifiers by either removing those console.log
calls or wrapping them behind a dev-only check (e.g., if (isDev) ...) or using a
privacy-safe logger that redacts/masks phone numbers, beneficiaryRegID and
parentRegID before logging, and ensure any error display via
confirmationService.alert does not expose sensitive fields from the error
payload.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
📋 Description
JIRA ID:
Moving 3.6.1 to main
✅ Type of Change
Summary by CodeRabbit
New Features
Improvements