AMM-1567 get ben family details changes #18
Conversation
…o abdmFacility
WalkthroughThe Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Router
participant BeneficiaryDetailsComponent
participant Backend
User->>Router: Navigates to beneficiary details route (with params)
Router->>BeneficiaryDetailsComponent: Supplies route parameters
BeneficiaryDetailsComponent->>BeneficiaryDetailsComponent: Extracts beneficiaryID and beneficiaryRegID from params
BeneficiaryDetailsComponent->>Backend: Requests beneficiary details using beneficiaryID
Backend-->>BeneficiaryDetailsComponent: Returns beneficiary details
BeneficiaryDetailsComponent->>Backend: Requests beneficiary image using beneficiaryRegID
Backend-->>BeneficiaryDetailsComponent: Returns beneficiary image
Possibly related PRs
Poem
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/registrar/beneficiary-details/beneficiary-details.component.ts (1)
169-171: Consider using optional chaining for additional safety.While the current null checks for
dataanddata.benImageare sufficient, the static analysis tool correctly identifies thatthis.beneficiarycould potentially be undefined when this assignment occurs.- this.beneficiary.benImage = data.benImage; + if (this.beneficiary) { + this.beneficiary.benImage = data.benImage; + }Alternatively, you could use optional chaining:
- this.beneficiary.benImage = data.benImage; + if (this.beneficiary) this.beneficiary.benImage = data.benImage;🧰 Tools
🪛 Biome (1.9.4)
[error] 169-169: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/registrar/beneficiary-details/beneficiary-details.component.ts(3 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
src/registrar/beneficiary-details/beneficiary-details.component.ts
[error] 169-169: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (2)
src/registrar/beneficiary-details/beneficiary-details.component.ts (2)
33-36: LGTM! Good formatting improvement.The import statement formatting with line breaks enhances readability.
80-80:Details
✅ Verification successful
Verify that the simplified conditional check doesn't introduce bugs.
The simplified truthy check
if (benFlowID)will now treat values like0,false, or empty string as falsy. Ensure that these values shouldn't be considered validbenFlowIDvalues in your system.
🏁 Script executed:
#!/bin/bash # Description: Search for benFlowID usage patterns to understand valid values # Expect: Find patterns showing what values benFlowID can have echo "Searching for benFlowID assignment and usage patterns..." rg -A 3 -B 3 "benFlowID.*=" --type ts echo "---" rg -A 3 -B 3 "benFlowID.*(!==|===|==|!=)" --type tsLength of output: 2357
Conditional check for
benFlowIDis safeThe
benFlowIDvalue comes fromsessionStorage.getItem('benFlowID')androute.params, both of which return either astringornull. In JavaScript, the only falsy values here arenulland''(empty string), so:
if (benFlowID)correctly skips when there’s no ID or it’s empty- Valid IDs such as
"0","false", or any non-empty string remain truthyNo unintended exclusions will occur—no changes needed.
| 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) { | ||
| this.beneficiary.benImage = data.benImage; | ||
| } | ||
| }); | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Good change to use route parameters, but fix the confusing variable naming.
The switch from session storage to route parameters is a good improvement for component isolation and testability. However, there's a naming issue that should be addressed:
- const benFlowID: any = param['beneficiaryRegID'];
+ const beneficiaryRegID: any = param['beneficiaryRegID'];
this.beneficiaryDetailsService
- .getBeneficiaryImage(benFlowID)
+ .getBeneficiaryImage(beneficiaryRegID)
.subscribe((data: any) => {The variable name benFlowID is misleading when it actually contains beneficiaryRegID. This could cause confusion for future developers.
🧰 Tools
🪛 Biome (1.9.4)
[error] 169-169: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🤖 Prompt for AI Agents
In src/registrar/beneficiary-details/beneficiary-details.component.ts between
lines 138 and 173, rename the variable benFlowID to beneficiaryRegID or a
similarly clear name that accurately reflects it holds the beneficiaryRegID from
route parameters. This will improve code clarity and prevent confusion about the
variable's purpose.
📋 Description
JIRA ID: AMM-1567
get ben family details changes
✅ Type of Change
ℹ️ Additional Information
Please describe how the changes were tested, and include any relevant screenshots, logs, or other information that provides additional context.
Summary by CodeRabbit
Bug Fixes
Style