Abdm facility and version 3 changes #52
Conversation
|
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 eslint
npm warn config production Use WalkthroughThis pull request introduces updates to multiple files across the application, focusing on ABHA (Ayushman Bharat Health Account) functionality. The changes include modifications to the Changes
Sequence DiagramsequenceDiagram
participant User
participant WorkareaComponent
participant RegistrarService
User->>WorkareaComponent: Initiate Health ID Retrieval
WorkareaComponent->>RegistrarService: Get Mapped ABDM Facility
RegistrarService-->>WorkareaComponent: Return Facility Details
WorkareaComponent->>WorkareaComponent: Save Facility for Visit
WorkareaComponent->>WorkareaComponent: Retrieve Health ID Details
Suggested Reviewers
Poem
Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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: 4
🧹 Nitpick comments (6)
src/app/app-modules/nurse-doctor/workarea/workarea.component.ts (3)
3255-3255: Remove unnecessary console.log statements.Avoid using
console.login production code. Consider removing it or using a proper logging mechanism.Apply this diff:
- console.log('workLocationId', workLocationId);
3266-3270: Use appropriate method for displaying error messages.Using
confirmationService.confirm()for error messages may confuse users. Consider usingconfirmationService.alert()to display errors.Apply this diff:
- this.confirmationService.confirm(res.errorMessage, 'info'); + this.confirmationService.alert(res.errorMessage, 'error');
3287-3287: Remove unnecessary console.log statements.Avoid using
console.login production code. Consider removing it or using a proper logging mechanism.Apply this diff:
- console.log('Abdm saved successfully');src/app/app.module.ts (1)
34-35: Avoid redundant provider declaration for RegistrarService.If
RegistrarServiceis already provided byRegistrationModule, it doesn't need to be added separately to theprovidersarray inAppModule. This prevents potential duplication and ensures a single instance of the service.src/app/app-modules/core/components/common-dialog/common-dialog.component.html (1)
51-56: Remove redundant event binding on the button.Using both
(click)="dialogRef.close(true)"andmat-dialog-closeis redundant. Since you're already handling the click event to close the dialog and pass a value, you can removemat-dialog-close.Apply this diff:
<button class="full-width-login button-ok" (click)="dialogRef.close(true)" - mat-dialog-close cdkFocusInitial >src/assets/English.json (1)
1722-1737: Consider improving message clarity and consistency.
- Inconsistent message formatting:
- "ABHA Search Mode*" has an asterisk while other similar keys don't
- Inconsistent capitalization in "Please Enter" vs "Please enter"
- Unclear messages:
- "Issue in printing ABHA Card" could be more specific about the error
Consider these improvements:
- "abhaSearchMode": "ABHA Search Mode*", + "abhaSearchMode": "ABHA Search Mode", - "enterCorrectAuthIdForAuthMode": "Please Enter correct Authentication ID for choosen AuthMode", + "enterCorrectAuthIdForAuthMode": "Please enter correct Authentication ID for chosen Auth Mode", - "issueInAbhaCard": "Issue in printing ABHA Card", + "issueInAbhaCard": "Unable to print ABHA Card. Please try again.",
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
Common-UI(1 hunks)src/app/app-modules/core/components/common-dialog/common-dialog.component.html(1 hunks)src/app/app-modules/nurse-doctor/workarea/workarea.component.ts(4 hunks)src/app/app.module.ts(3 hunks)src/assets/Assamese.json(1 hunks)src/assets/English.json(1 hunks)src/assets/Hindi.json(1 hunks)src/environments/environment.dev.ts(1 hunks)src/environments/environment.development.ts(1 hunks)src/environments/environment.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- Common-UI
🔇 Additional comments (5)
src/app/app-modules/nurse-doctor/workarea/workarea.component.ts (2)
61-61: Import RegistrarService for ABDM facility mapping.
The import of RegistrarService is necessary for fetching the mapped ABDM facility details.
3272-3275: Verify the necessity of calling saveAbdmFacilityForVisit() after errors.
saveAbdmFacilityForVisit() is called even when there is an error in fetching the mapped ABDM facility. Please verify if this is the intended behavior, as it might attempt to save a null abdmFacilityId.
src/environments/environment.ts (1)
72-72: Update 'abhaExtension' value for correct configuration.
The change from @abdm to @sbx reflects the updated configuration. Ensure this value matches the required environment setup.
src/environments/environment.development.ts (1)
69-69: LGTM! Configuration is consistent
The ABHA extension change to @sbx is consistent with the dev environment configuration.
src/assets/Assamese.json (1)
1715-1730: LGTM! Translations are consistent with English counterparts.
The new ABHA-related translations maintain proper context and follow the established localization patterns.
| abdmFacilityId = null; | ||
| abdmFacilityName = null; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add type annotations to class properties.
Please specify the types for abdmFacilityId and abdmFacilityName to enhance type safety and code readability.
Apply this diff to add type annotations:
- abdmFacilityId = null;
- abdmFacilityName = null;
+ abdmFacilityId: string | null = null;
+ abdmFacilityName: string | null = null;📝 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.
| abdmFacilityId = null; | |
| abdmFacilityName = null; | |
| abdmFacilityId: string | null = null; | |
| abdmFacilityName: string | null = null; |
There was a problem hiding this comment.
Its maintained like that all over the project. hence following the same
There was a problem hiding this comment.
@helenKaryamsetty, understood. Maintaining consistency with the existing codebase is important. Thank you for the clarification.
✏️ Learnings added
Learnt from: helenKaryamsetty
PR: PSMRI/TM-UI#52
File: src/app/app-modules/nurse-doctor/workarea/workarea.component.ts:181-182
Timestamp: 2024-12-16T11:12:21.288Z
Learning: In the `src/app/app-modules/nurse-doctor/workarea/workarea.component.ts` file, and throughout the Angular project, class properties are declared without explicit type annotations to maintain consistency.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
| getMappedAbdmFacility() { | ||
| const locationData: any = localStorage.getItem('loginDataResponse'); | ||
| const jsonLoccationData = JSON.parse(locationData); | ||
| let workLocationId: any; | ||
| if (jsonLoccationData?.previlegeObj[0]?.roles) { | ||
| const roles = jsonLoccationData?.previlegeObj[0]?.roles; | ||
| roles.find((item: any) => { | ||
| item.RoleName.toLowerCase() === 'doctor'; | ||
| workLocationId = item.workingLocationID; | ||
| }); | ||
| } | ||
| console.log('workLocationId', workLocationId); |
There was a problem hiding this comment.
Fix logical error in roles.find() callback function.
The callback function in roles.find() does not return a boolean value, which means the find method will not work as expected. Additionally, there are typos in variable names.
Apply this diff to correct the issue:
- const jsonLoccationData = JSON.parse(locationData);
+ const jsonLocationData = JSON.parse(locationData);
- if (jsonLoccationData?.previlegeObj[0]?.roles) {
- const roles = jsonLoccationData?.previlegeObj[0]?.roles;
+ if (jsonLocationData?.privilegeObj[0]?.roles) {
+ const roles = jsonLocationData?.privilegeObj[0]?.roles;
- roles.find((item: any) => {
- item.RoleName.toLowerCase() === 'doctor';
- workLocationId = item.workingLocationID;
- });
+ const doctorRole = roles.find((item: any) => item.RoleName.toLowerCase() === 'doctor');
+ if (doctorRole) {
+ workLocationId = doctorRole.workingLocationID;
+ }📝 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.
| getMappedAbdmFacility() { | |
| const locationData: any = localStorage.getItem('loginDataResponse'); | |
| const jsonLoccationData = JSON.parse(locationData); | |
| let workLocationId: any; | |
| if (jsonLoccationData?.previlegeObj[0]?.roles) { | |
| const roles = jsonLoccationData?.previlegeObj[0]?.roles; | |
| roles.find((item: any) => { | |
| item.RoleName.toLowerCase() === 'doctor'; | |
| workLocationId = item.workingLocationID; | |
| }); | |
| } | |
| console.log('workLocationId', workLocationId); | |
| getMappedAbdmFacility() { | |
| const locationData: any = localStorage.getItem('loginDataResponse'); | |
| const jsonLocationData = JSON.parse(locationData); | |
| let workLocationId: any; | |
| if (jsonLocationData?.privilegeObj[0]?.roles) { | |
| const roles = jsonLocationData?.privilegeObj[0]?.roles; | |
| const doctorRole = roles.find((item: any) => item.RoleName.toLowerCase() === 'doctor'); | |
| if (doctorRole) { | |
| workLocationId = doctorRole.workingLocationID; | |
| } | |
| } | |
| console.log('workLocationId', workLocationId); |
There was a problem hiding this comment.
I dont need a booelan keyword but data is needed and i want to use find() method only for my code purpose
There was a problem hiding this comment.
@helenKaryamsetty, understood. If you prefer using the find() method in this way for your code, that's perfectly fine. Let me know if there's anything else I can assist with.
✏️ Learnings added
Learnt from: helenKaryamsetty
PR: PSMRI/TM-UI#52
File: src/app/app-modules/nurse-doctor/workarea/workarea.component.ts:3244-3255
Timestamp: 2024-12-16T11:11:54.647Z
Learning: In `src/app/app-modules/nurse-doctor/workarea/workarea.component.ts`, the developer prefers to use the `find()` method without explicitly returning a boolean in the callback function.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
| @@ -66,7 +66,7 @@ export const environment = { | |||
| RBSTest: `RBS Test`, | |||
| visualAcuityTest: `Visual Acuity Test`, | |||
| haemoglobinTest: `Haemoglobin Test`, | |||
| abhaExtension: `@abdm`, | |||
| abhaExtension: `@sbx`, | |||
There was a problem hiding this comment.
💡 Codebase verification
Inconsistent ABHA extension configuration detected across environments
The ABHA extension configuration shows a mix of values:
- Development environments (dev, development, base) use
@sbx - Production, test, and CI environments use
@abdm
This inconsistency might be intentional as it separates sandbox (@sbx) for development from production (@abdm), but the test environment using @abdm instead of @sbx seems unusual for a testing setup.
src/environments/environment.test.ts: Consider changing@abdmto@sbxto maintain consistency with other non-production environmentssrc/environments/environment.ci.ts.template: Consider changing@abdmto@sbxif CI environment is used for testing
🔗 Analysis chain
Verify the ABHA extension configuration change
The ABHA extension has been changed from @abdm to @sbx. This appears to be switching from production to sandbox environment.
Let's verify if this change is consistent across all environment files and check for any related configuration:
Let's check the actual values of abhaExtension across these environment files to ensure consistency.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check ABHA extension configuration across environment files
# Expected: Consistent @sbx configuration in all environment files
echo "Checking ABHA extension configuration across environment files..."
rg -l "abhaExtension.*@(abdm|sbx)" src/environments/
Length of output: 425
Script:
#!/bin/bash
# Check the actual values of abhaExtension across environment files
echo "Content of abhaExtension in environment files:"
rg "abhaExtension.*@(abdm|sbx)" src/environments/
Length of output: 537
There was a problem hiding this comment.
@coderabbitai i have added those values based on environments... in dev environment it is sbx and prod environment it is abdm
There was a problem hiding this comment.
@helenKaryamsetty, thank you for the clarification. I'll note that you've intentionally set abhaExtension to @sbx in development environments and @abdm in production environments to differentiate between them.
✏️ Learnings added
Learnt from: helenKaryamsetty
PR: PSMRI/TM-UI#52
File: src/environments/environment.dev.ts:69-69
Timestamp: 2024-12-16T11:10:14.588Z
Learning: In the `environment` files of the project, the `abhaExtension` configuration is intentionally set to `@sbx` in development environments (e.g., `src/environments/environment.dev.ts`, `src/environments/environment.development.ts`) and to `@abdm` in production environments (e.g., `src/environments/environment.prod.ts`). This setup differentiates between sandbox and production environments.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
src/app/app-modules/nurse-doctor/workarea/workarea.component.ts
Outdated
Show resolved
Hide resolved
|
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/app/app-modules/nurse-doctor/workarea/workarea.component.ts(4 hunks)
🔇 Additional comments (3)
src/app/app-modules/nurse-doctor/workarea/workarea.component.ts (3)
181-182: LGTM!
The properties are correctly initialized to store ABDM facility details, following the project's conventions.
3279-3296: LGTM!
The method follows good practices for saving ABDM facility details:
- Clean request object construction
- Proper error handling
- Appropriate user feedback
Line range hint 3226-3243: LGTM!
The integration of ABDM facility mapping with health ID linking follows a logical sequence and ensures proper data availability.
| getMappedAbdmFacility() { | ||
| const locationData: any = localStorage.getItem('loginDataResponse'); | ||
| const jsonLoccationData = JSON.parse(locationData); | ||
| let workLocationId: any; | ||
| if (jsonLoccationData?.previlegeObj[0]?.roles) { | ||
| const roles = jsonLoccationData?.previlegeObj[0]?.roles; | ||
| roles.find((item: any) => { | ||
| item.RoleName.toLowerCase() === 'doctor'; | ||
| workLocationId = item.workingLocationID; | ||
| }); | ||
| } | ||
| console.log('workLocationId', workLocationId); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fix typos and add error handling for JSON parsing.
The code has the following issues:
- Variable name typo:
jsonLoccationDatashould bejsonLocationData - Property path typo:
previlegeObjshould beprivilegeObj - Missing error handling for JSON.parse
Apply this diff to fix the issues:
- const jsonLoccationData = JSON.parse(locationData);
+ try {
+ const jsonLocationData = JSON.parse(locationData);
+
+ if (jsonLocationData?.privilegeObj[0]?.roles) {
+ const roles = jsonLocationData?.privilegeObj[0]?.roles;
+ roles.find((item: any) => {
+ item.RoleName.toLowerCase() === 'doctor';
+ workLocationId = item.workingLocationID;
+ });
+ }
+ } catch (error) {
+ console.error('Error parsing location data:', error);
+ return;
+ }📝 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.
| getMappedAbdmFacility() { | |
| const locationData: any = localStorage.getItem('loginDataResponse'); | |
| const jsonLoccationData = JSON.parse(locationData); | |
| let workLocationId: any; | |
| if (jsonLoccationData?.previlegeObj[0]?.roles) { | |
| const roles = jsonLoccationData?.previlegeObj[0]?.roles; | |
| roles.find((item: any) => { | |
| item.RoleName.toLowerCase() === 'doctor'; | |
| workLocationId = item.workingLocationID; | |
| }); | |
| } | |
| console.log('workLocationId', workLocationId); | |
| getMappedAbdmFacility() { | |
| const locationData: any = localStorage.getItem('loginDataResponse'); | |
| try { | |
| const jsonLocationData = JSON.parse(locationData); | |
| if (jsonLocationData?.privilegeObj[0]?.roles) { | |
| const roles = jsonLocationData?.privilegeObj[0]?.roles; | |
| roles.find((item: any) => { | |
| item.RoleName.toLowerCase() === 'doctor'; | |
| workLocationId = item.workingLocationID; | |
| }); | |
| } | |
| } catch (error) { | |
| console.error('Error parsing location data:', error); | |
| return; | |
| } | |
| console.log('workLocationId', workLocationId); |
| if (jsonLoccationData?.previlegeObj[0]?.roles) { | ||
| const roles = jsonLoccationData?.previlegeObj[0]?.roles; | ||
| roles.find((item: any) => { | ||
| item.RoleName.toLowerCase() === 'doctor'; |
There was a problem hiding this comment.
@drtechie Yes, we are fetching the role mapped to userName and only giving carecontext facility details if the role is doctor.
📋 Description
JIRA ID: AMM-854, 1017,1016
Please provide a summary of the change and the motivation behind it. Include relevant context and details.
✅ 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
Release Notes
New Features
Bug Fixes
Chores