AMM-1536 Tele consultation configuration changes in work location mapping screen #89
Conversation
WalkthroughThe changes generalize the "eSanjeevani" feature to a broader "Tele Consultation" concept in the work location mapping UI and logic, update API endpoint paths in environment files, and remove the file replacement configuration from the Angular development build. Pagination handling in the component is also refined. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant WorkLocationMappingComponent
participant BackendAPI
User->>WorkLocationMappingComponent: Selects service line or role
WorkLocationMappingComponent->>WorkLocationMappingComponent: Calls teleConsultationSaveFunction
User->>WorkLocationMappingComponent: Chooses Tele Consultation option (ESanjeevani, Swymed, Not Required)
WorkLocationMappingComponent->>WorkLocationMappingComponent: Updates teleConsultation variable
User->>WorkLocationMappingComponent: Adds or edits work location mapping
WorkLocationMappingComponent->>BackendAPI: Sends mapping data with teleConsultation value
BackendAPI-->>WorkLocationMappingComponent: Responds with result
WorkLocationMappingComponent-->>User: Updates UI tables and forms
Possibly related PRs
Suggested reviewers
Poem
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 error code ERESOLVE ✨ 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: 3
🔭 Outside diff range comments (1)
src/app/app-provider-admin/provider-admin/activities/work-location-mapping/work-location-mapping.component.ts (1)
841-1085: Refactor the addWorkLocation method to improve maintainability.This method is over 240 lines long and handles multiple responsibilities. Consider breaking it down into smaller, service-specific methods.
Consider refactoring like this:
addWorkLocation(objectToBeAdded: any, role: any) { const serviceName = objectToBeAdded.serviceline.serviceName; switch(serviceName) { case '1097': this.handle1097Service(objectToBeAdded); break; case 'ECD': this.handleECDService(objectToBeAdded); break; case 'HWC': this.handleHWCService(objectToBeAdded); break; default: this.handleDefaultService(objectToBeAdded); } } private handle1097Service(objectToBeAdded: any): void { // Extract 1097-specific logic here } private handleECDService(objectToBeAdded: any): void { // Extract ECD-specific logic here } // ... other service-specific methodsThis would make the code more modular and easier to test and maintain.
🧹 Nitpick comments (4)
src/app/app-provider-admin/provider-admin/activities/work-location-mapping/work-location-mapping.component.html (1)
628-651: Invoke teleConsultation logic on edit selection
CurrentlyteleConsultationEditSaveFunctionis bound to themat-optionclick. For consistent behavior, consider moving this call to theselectionChangeevent on themat-selectinstead of individual options.src/app/app-provider-admin/provider-admin/activities/work-location-mapping/work-location-mapping.component.ts (3)
130-131: Consider renaming ViewChild references for clarity.The paginator names
paginatorFirstandpaginatorSecondare not descriptive. Consider using more meaningful names that indicate their purpose.-@ViewChild('paginatorFirst') paginatorFirst!: MatPaginator; -@ViewChild('paginatorSecond') paginatorSecond!: MatPaginator; +@ViewChild('mappedLocationsPaginator') mappedLocationsPaginator!: MatPaginator; +@ViewChild('bufferArrayPaginator') bufferArrayPaginator!: MatPaginator;
229-230: Centralize paginator management to reduce code duplication.The paginator assignments are repeated throughout the code. Consider creating helper methods to manage pagination centrally.
Add these helper methods:
private updateMappedLocationsPaginator(): void { if (this.mappedLocationsPaginator) { this.filteredmappedWorkLocationsList.paginator = this.mappedLocationsPaginator; } } private updateBufferArrayPaginator(): void { if (this.bufferArrayPaginator) { this.bufferArray.paginator = this.bufferArrayPaginator; } } private refreshPaginators(): void { this.updateMappedLocationsPaginator(); this.updateBufferArrayPaginator(); }Then replace all paginator assignments with calls to these methods.
Also applies to: 320-320, 552-552, 567-567, 584-584, 935-935, 948-948, 953-953, 1020-1020, 1080-1080, 1164-1164, 1186-1186, 1194-1194, 1202-1202, 1286-1286, 1310-1310, 1904-1904, 1955-1955
2227-2251: Rename variable to maintain consistency with teleConsultation refactoring.The variable
roleSanjArrystill references the old "Sanj" naming convention. This should be updated for consistency.-const roleSanjArry: any = []; +const roleNamesArray: any = []; if (this.Role) { this.Role.filter((item: any) => { - roleSanjArry.push(item.roleName.toLowerCase()); + roleNamesArray.push(item.roleName.toLowerCase()); }); } if ( this.Serviceline.serviceName === 'HWC' && - (roleSanjArry.includes('nurse') || roleSanjArry.includes('doctor')) + (roleNamesArray.includes('nurse') || roleNamesArray.includes('doctor')) ) {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
angular.json(1 hunks)src/app/app-provider-admin/provider-admin/activities/work-location-mapping/work-location-mapping.component.html(10 hunks)src/app/app-provider-admin/provider-admin/activities/work-location-mapping/work-location-mapping.component.ts(36 hunks)src/environments/environment.prod.ts(1 hunks)src/environments/environment.test.ts(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Build
🔇 Additional comments (11)
src/environments/environment.prod.ts (1)
30-33: Update base URL segments and verify removal of old ones
The API paths have been updated toadmin-api/,common-api/, andfhir-api/. Ensure no legacyadminapi-v1.0,commonapi-v1.0, orfhirapi-v1.0references remain.
Run:#!/bin/bash # Verify that no old versioned API paths persist rg -H "adminapi-v1.0|commonapi-v1.0|fhirapi-v1.0"src/environments/environment.test.ts (1)
30-33: Align test environment URLs with production
The test config now uses the sameadmin-api/,common-api/, andfhir-api/suffixes. Confirm these endpoints are reachable in the test environment and check for any leftover v1.0 references.
Run:#!/bin/bash # Search for stale API references in test config rg -H "adminapi-v1.0|commonapi-v1.0|fhirapi-v1.0"angular.json (1)
63-70: Confirm development build environment file mapping
Removing thefileReplacementsfor development meansenvironment.tswill now remain unaltered in dev builds. Verify thatsrc/environments/environment.tscontains the correct development API URLs and update any developer documentation accordingly.src/app/app-provider-admin/provider-admin/activities/work-location-mapping/work-location-mapping.component.html (8)
170-177: Introduce separate paginator for mapped list
Adding#paginatorFirstisolates pagination for the filtered mapped locations. Ensure the component class has@ViewChild('paginatorFirst') paginatorFirst: MatPaginator;and wires it up inngAfterViewInit().
233-236: Trigger teleConsultation state on service change
The new calls toresetRole()andteleConsultationSaveFunction('Serviceline', Serviceline)reset dependent selections and propagate tele consultation defaults on service-line change. ConfirmteleConsultationSaveFunctioncorrectly updatesteleConsultationFlagand resets other form fields.
373-380: Cascade teleConsultation on role selection
InvokingteleConsultationSaveFunction('Role', Role)when roles change ensures the radio-group options reflect the current context. Verify this handles both single and multi-role scenarios and togglesteleConsultationFlagas intended.
389-410: Add Tele Consultation options UI
The radio-group forteleConsultationreplaces the old eSanjeevani checkbox, supportingESanjeevani,Swymed, andNot Required. EnsureteleConsultationFlagcontrols visibility and that the[(ngModel)]="teleConsultation"binding aligns with the component property.
436-455: Include add button within form for new mappings
The FAB now appears only when the form is valid and roles are selected. Confirm that theaddWorkLocationmethod signature in the component has been updated to accept and forward theteleConsultationvalue fromworkplaceform.value.
532-537: Display teleConsultation in buffer table
The newTeleConsultationcolumn surfaces the selected option in the buffered list. VerifydisplayedColumnsTable2includes'TeleConsultation'and that eachbufferArray.dataobject carries ateleConsultationproperty.
563-568: Add second paginator for buffered entries
The#paginatorSecondreference enables independent pagination of the buffer table. Ensure the component is injecting this via@ViewChild('paginatorSecond')and initializing it appropriately.
817-838: Add Tele Consultation options in edit form
The edit form radio-group bound toteleConsultationEditmirrors the create form behavior. ConfirmteleConsultationEditFlagis set on load and thatteleConsultationEditholds the existing mapping value.
| teleConsultation: | ||
| objectToBeAdded.serviceline.serviceName === 'HWC' && | ||
| objectToBeAdded.role[a].roleName.toLowerCase() === 'nurse' && | ||
| this.isSanjeevani | ||
| ? true | ||
| : false, | ||
| (objectToBeAdded.role[a].roleName.toLowerCase() === 'nurse' || | ||
| objectToBeAdded.role[a].roleName.toLowerCase() === | ||
| 'doctor') && | ||
| this.teleConsultation | ||
| ? this.teleConsultation | ||
| : null, | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Extract duplicated teleConsultation logic into a helper method.
The teleConsultation assignment logic is repeated four times with identical conditions. This violates the DRY principle and makes the code harder to maintain.
Add a helper method at the class level:
private getTeleConsultationValue(serviceName: string, roleName: string): string | null {
return serviceName === 'HWC' &&
(roleName.toLowerCase() === 'nurse' || roleName.toLowerCase() === 'doctor') &&
this.teleConsultation
? this.teleConsultation
: null;
}Then replace all occurrences with:
-teleConsultation:
- objectToBeAdded.serviceline.serviceName === 'HWC' &&
- (objectToBeAdded.role[a].roleName.toLowerCase() === 'nurse' ||
- objectToBeAdded.role[a].roleName.toLowerCase() === 'doctor') &&
- this.teleConsultation
- ? this.teleConsultation
- : null,
+teleConsultation: this.getTeleConsultationValue(
+ objectToBeAdded.serviceline.serviceName,
+ objectToBeAdded.role[a].roleName
+),Also applies to: 996-1005, 1032-1040, 1058-1067
🤖 Prompt for AI Agents
In
src/app/app-provider-admin/provider-admin/activities/work-location-mapping/work-location-mapping.component.ts
around lines 970 to 978, 996 to 1005, 1032 to 1040, and 1058 to 1067, the
teleConsultation assignment logic is duplicated with identical conditions. To
fix this, create a private helper method in the class that takes serviceName and
roleName as parameters and returns the teleConsultation value or null based on
the condition. Then replace all repeated teleConsultation assignment code blocks
with calls to this new helper method to adhere to the DRY principle and improve
maintainability.
There was a problem hiding this comment.
The logic was part of the existing code and I have only restructured it without introducing new duplications.
There was a problem hiding this comment.
@devikasuresh20 You're absolutely right! I can see that you were refactoring the existing isSanjeevani logic to the new teleConsultation approach, and the duplication pattern was likely already present in the original code structure.
Thank you for clarifying that - it's completely reasonable to focus on the core refactoring (eSanjeevani → teleConsultation) without addressing pre-existing code patterns in the same PR. The suggestion remains valuable for future code improvements, but I understand it's outside the scope of your current changes.
Your refactoring from the boolean isSanjeevani to the more flexible string-based teleConsultation that supports multiple options ('ESanjeevani', 'Swymed', 'Not Required') is a solid improvement to the functionality! 🐰
/| /|
( :v: )
|(_)|
✏️ Learnings added
Learnt from: devikasuresh20
PR: PSMRI/ADMIN-UI#89
File: src/app/app-provider-admin/provider-admin/activities/work-location-mapping/work-location-mapping.component.ts:970-978
Timestamp: 2025-06-23T11:04:02.816Z
Learning: When users are refactoring existing code logic (like changing from isSanjeevani boolean to teleConsultation string values), pre-existing code duplication patterns should be acknowledged as separate from the refactoring scope, and suggestions for addressing such duplication should be framed as future improvements rather than requirements for the current PR.
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.
| teleConsultationFlag = false; | ||
| teleConsultation: any; | ||
| teleConsultationEditFlag = false; | ||
| esanjFlag = false; | ||
| eSanjeevaniEdit = false; | ||
| teleConsultationEdit: any; | ||
| foundDuplicate = false; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve type safety and eliminate magic strings.
The code uses any type extensively and contains magic strings that should be constants or enums.
Consider creating enums and interfaces:
enum ServiceName {
HWC = 'HWC',
ECD = 'ECD',
HELPLINE = '1097',
FLW = 'FLW',
TM = 'TM',
MMU = 'MMU'
}
enum RoleName {
NURSE = 'nurse',
DOCTOR = 'doctor',
SUPERVISOR = 'supervisor',
TC_SPECIALIST = 'TC Specialist'
}
enum TeleConsultationType {
ESANJEEVANI = 'ESanjeevani',
SWYMED = 'Swymed',
NOT_REQUIRED = 'Not Required'
}
interface TeleConsultationConfig {
teleConsultationFlag: boolean;
teleConsultation: TeleConsultationType | null;
teleConsultationEditFlag: boolean;
teleConsultationEdit: TeleConsultationType | null;
}This would make the code more maintainable and less error-prone.
Also applies to: 970-978, 2236-2238
🤖 Prompt for AI Agents
In
src/app/app-provider-admin/provider-admin/activities/work-location-mapping/work-location-mapping.component.ts
around lines 90 to 95, replace the use of 'any' types and magic strings with
well-defined enums and interfaces to improve type safety and maintainability.
Define enums for service names, role names, and teleconsultation types, and
create an interface for teleconsultation configuration. Update the variables
teleConsultation and teleConsultationEdit to use the TeleConsultationType enum
or null instead of any, and replace all magic string literals with the
corresponding enum values. Apply similar changes to lines 970-978 and 2236-2238
as well.
There was a problem hiding this comment.
These variables and types are part of existing code and my changes were limited to configuration updates without altering the type structure.
There was a problem hiding this comment.
@devikasuresh20 I understand completely! You're absolutely right to limit the scope to configuration changes only. Keeping the existing type structure intact while focusing specifically on the tele consultation configuration updates is a sensible approach for this PR. It maintains consistency with the existing codebase and keeps the changes focused on the specific feature requirement.
Thank you for clarifying your approach - it's good practice to keep refactoring separate from feature changes.
🐰✨ curiosity satisfied
✏️ Learnings added
Learnt from: devikasuresh20
PR: PSMRI/ADMIN-UI#89
File: src/app/app-provider-admin/provider-admin/activities/work-location-mapping/work-location-mapping.component.ts:90-95
Timestamp: 2025-06-23T11:05:44.626Z
Learning: The user prefers to limit PR scope to specific feature changes without altering existing type structures, keeping configuration updates separate from broader refactoring efforts.
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.
| !this.editMode && | ||
| parseInt(mappedWorkLocations.stateID) === this.State.stateID && | ||
| parseInt(mappedWorkLocations.workingDistrictID) === | ||
| this.District.districtID && | ||
| !mappedWorkLocations.userServciceRoleDeleted | ||
| ) { | ||
| this.existingRoles.push(mappedWorkLocations.roleID); | ||
| } else if ( | ||
| (serviceID === 2 || serviceID === 4) && | ||
| this.editMode && | ||
| parseInt(mappedWorkLocations.stateID) === this.stateID_duringEdit && | ||
| parseInt(mappedWorkLocations.workingDistrictID) === | ||
| this.district_duringEdit && | ||
| !mappedWorkLocations.userServciceRoleDeleted | ||
| ) { | ||
| this.existingRoles.push(mappedWorkLocations.roleID); | ||
| } else if ( |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Refactor duplicated role existence logic.
The role existence check logic is duplicated for edit mode and non-edit mode. Consider extracting this into a helper method to improve maintainability.
-if (
- (serviceID === 2 || serviceID === 4) &&
- !this.editMode &&
- parseInt(mappedWorkLocations.stateID) === this.State.stateID &&
- parseInt(mappedWorkLocations.workingDistrictID) ===
- this.District.districtID &&
- !mappedWorkLocations.userServciceRoleDeleted
-) {
- this.existingRoles.push(mappedWorkLocations.roleID);
-} else if (
- (serviceID === 2 || serviceID === 4) &&
- this.editMode &&
- parseInt(mappedWorkLocations.stateID) === this.stateID_duringEdit &&
- parseInt(mappedWorkLocations.workingDistrictID) ===
- this.district_duringEdit &&
- !mappedWorkLocations.userServciceRoleDeleted
-) {
- this.existingRoles.push(mappedWorkLocations.roleID);
+const stateID = this.editMode ? this.stateID_duringEdit : this.State?.stateID;
+const districtID = this.editMode ? this.district_duringEdit : this.District?.districtID;
+
+if (
+ (serviceID === 2 || serviceID === 4) &&
+ parseInt(mappedWorkLocations.stateID) === stateID &&
+ parseInt(mappedWorkLocations.workingDistrictID) === districtID &&
+ !mappedWorkLocations.userServciceRoleDeleted
+) {
+ this.existingRoles.push(mappedWorkLocations.roleID);📝 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.
| !this.editMode && | |
| parseInt(mappedWorkLocations.stateID) === this.State.stateID && | |
| parseInt(mappedWorkLocations.workingDistrictID) === | |
| this.District.districtID && | |
| !mappedWorkLocations.userServciceRoleDeleted | |
| ) { | |
| this.existingRoles.push(mappedWorkLocations.roleID); | |
| } else if ( | |
| (serviceID === 2 || serviceID === 4) && | |
| this.editMode && | |
| parseInt(mappedWorkLocations.stateID) === this.stateID_duringEdit && | |
| parseInt(mappedWorkLocations.workingDistrictID) === | |
| this.district_duringEdit && | |
| !mappedWorkLocations.userServciceRoleDeleted | |
| ) { | |
| this.existingRoles.push(mappedWorkLocations.roleID); | |
| } else if ( | |
| const stateID = this.editMode | |
| ? this.stateID_duringEdit | |
| : this.State?.stateID; | |
| const districtID = this.editMode | |
| ? this.district_duringEdit | |
| : this.District?.districtID; | |
| if ( | |
| (serviceID === 2 || serviceID === 4) && | |
| parseInt(mappedWorkLocations.stateID) === stateID && | |
| parseInt(mappedWorkLocations.workingDistrictID) === districtID && | |
| !mappedWorkLocations.userServciceRoleDeleted | |
| ) { | |
| this.existingRoles.push(mappedWorkLocations.roleID); | |
| } else if ( |
🤖 Prompt for AI Agents
In
src/app/app-provider-admin/provider-admin/activities/work-location-mapping/work-location-mapping.component.ts
around lines 411 to 427, the logic checking role existence for edit mode and
non-edit mode is duplicated. Refactor by extracting this repeated conditional
logic into a single helper method that accepts parameters for mode and relevant
IDs, then use this method in the if-else blocks to push roleIDs to
existingRoles. This will improve code maintainability and reduce duplication.
There was a problem hiding this comment.
The logic was part of the existing code and I have only restructured it without introducing new duplications.
There was a problem hiding this comment.
@devikasuresh20 I understand your perspective. You're absolutely right that when working with existing code, sometimes it's more practical to maintain consistency with the current patterns rather than introducing broader refactoring changes. Your approach of restructuring the existing logic to handle the edit mode requirements makes sense in this context.
Thank you for the clarification!
(\ /)
( ._. )
o_(")(") *understanding rabbit*
devikasuresh20
changed the title
2 participants
📋 Description
JIRA ID: AMM-1536
Added changes for configuring tele consultation ( ESanjeevani or Swymed) from work location mapping screen.
✅ 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
New Features
Refactor
Chores