removed issanjeevani variable - #225
Conversation
WalkthroughThe changes replace the usage of the Boolean field and database column Changes
Sequence Diagram(s)sequenceDiagram
participant Controller as IEMRAdminController
participant Service as IEMRAdminUserServiceImpl
participant Mapping as UserServiceRoleMapping
participant Repo as UserRoleMappingRepository
participant DB as Database
Controller->>Service: Request user role mapping
Service->>Repo: getUserRoleMappingForUser(userID)
Repo->>DB: Query includes 'teleConsultation' field
DB-->>Repo: Returns user role mapping with 'teleConsultation'
Repo-->>Service: Mapping data
Service->>Mapping: initializeUserRoleMappingObjs(..., teleConsultation, ...)
Mapping-->>Service: UserServiceRoleMapping object
Service-->>Controller: User role mapping (with 'teleConsultation')
Suggested reviewers
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
🔭 Outside diff range comments (3)
src/main/java/com/iemr/common/service/users/IEMRAdminUserServiceImpl.java (2)
468-478: Guard length check must match new projection sizeArray element
object[11]is accessed but the guard still checks for>= 11.
Use>= 12to avoid a potentialArrayIndexOutOfBoundsException.- if (object != null && object.length >= 11) { + if (object != null && object.length >= 12) {
667-675: Same length-check issue ingetUserServiceRoleMappingReplicate the fix here:
- if (object != null && object.length >= 11) { + if (object != null && object.length >= 12) {src/main/java/com/iemr/common/data/users/UserServiceRoleMapping.java (1)
303-321: Parameter explosion – consider a value object
initializeUserRoleMappingObjsnow takes 13 positional parameters, which is error-prone (the recent rename already caused index mis-alignment). A builder or dedicated DTO would make future changes safer and self-documenting.Example sketch:
UserServiceRoleMapping mapping = UserServiceRoleMappingBuilder .ids(USRMappingID, UserID) .role(roleID, m_Role) .provider(providerServiceMapID, m_ProviderServiceMapping) .agent(agentID, agentPassword, inbound, outbound, teleConsultation) .location(workingLocationID, providerServiceAddressMapping) .build();
🧹 Nitpick comments (2)
src/main/java/com/iemr/common/controller/users/IEMRAdminController.java (1)
408-411: Null-safety for new field when building JSON
m_UserServiceRoleMapping.getTeleConsultation()can benull.
If the client relies on a non-null string, add a default (e.g.,"") or omit the key when null to avoid"teleConsultation":nullleaking into the payload.- roleObject.put("teleConsultation", m_UserServiceRoleMapping.getTeleConsultation()); + String tc = m_UserServiceRoleMapping.getTeleConsultation(); + if (tc != null) { + roleObject.put("teleConsultation", tc); + }src/main/java/com/iemr/common/data/users/Role.java (1)
96-104: TransientteleConsultationfield seems redundant
teleConsultationis now held inUserServiceRoleMappingand copied into JSON directly from there.
Keeping another copy insideRole(and marking it@Transient) increases object size and can get out-of-sync. Unless there is a clear future use-case, consider dropping it.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/main/java/com/iemr/common/controller/users/IEMRAdminController.java(1 hunks)src/main/java/com/iemr/common/data/users/Role.java(2 hunks)src/main/java/com/iemr/common/data/users/UserServiceRoleMapping.java(3 hunks)src/main/java/com/iemr/common/repository/users/UserRoleMappingRepository.java(1 hunks)src/main/java/com/iemr/common/service/users/IEMRAdminUserServiceImpl.java(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: style-check / checkstyle
- GitHub Check: Package-test
- GitHub Check: Analyze (java)
- GitHub Check: Build
🔇 Additional comments (1)
src/main/java/com/iemr/common/data/users/UserServiceRoleMapping.java (1)
112-115: Column definition – ensure DB schema matchesThe column is declared as
varchar(String), whereas the removedisSanjeevaniwas aboolean.
Confirm the DB migration alters the column type accordingly; otherwise Hibernate may fail on schema validation.
| @Query("select map.USRMappingID, map.UserID, map.RoleID, map.m_Role, map.providerServiceMapID, " | ||
| + "map.agentID, map.inbound, map.outbound, map.isSanjeevani, map.agentPassword, map.workingLocationID, map.providerServiceAddressMapping from " | ||
| + "map.agentID, map.inbound, map.outbound, map.teleConsultation, map.agentPassword, map.workingLocationID, map.providerServiceAddressMapping from " | ||
| + "UserServiceRoleMapping map left join map.m_ProviderServiceMapping psm left join psm.serviceProvider sp " |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Index shift introduces downstream risk – update consuming code accordingly
The projection now returns 12 columns (the new map.teleConsultation at position 9).
Down-stream loops in IEMRAdminUserServiceImpl still guard with object.length >= 11 but subsequently dereference object[11], which will throw ArrayIndexOutOfBoundsException if a shorter result is ever returned (e.g., due to LEFT JOIN nulls).
Either:
- if (object != null && object.length >= 11) {
+ if (object != null && object.length >= 12) {or switch to a DTO/result-class projection to remove this brittle positional coupling.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/main/java/com/iemr/common/repository/users/UserRoleMappingRepository.java
around lines 38 to 40, the query projection now returns 12 columns due to the
addition of map.teleConsultation, but downstream code in
IEMRAdminUserServiceImpl still checks for object length >= 11 and accesses
object[11], risking ArrayIndexOutOfBoundsException. To fix this, update the
downstream code to check for object length >= 12 before accessing object[11], or
refactor the query and consuming code to use a DTO or result-class projection to
avoid brittle positional indexing.
2 participants
📋 Description
JIRA ID: AMM-1654
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