GetBenHealthIdDetails API changes #64
Conversation
WalkthroughThe changes refactor the handling of health ID mappings by introducing Lombok's Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client
participant Service as HealthIDServiceImpl
participant Repo as BenHealthIDMappingRepo
Client->>Service: Request health ID details
Service->>Repo: Call getIsNewAbha(healthIdNumber)
Repo-->>Service: Return isNewAbha status
Service->>Service: Enrich health details with isNewAbha flag
Service->>Client: Return enriched health details
Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
🪧 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: 0
🧹 Nitpick comments (1)
src/main/java/com/wipro/fhir/repo/healthID/BenHealthIDMappingRepo.java (1)
79-80: New method for retrieving ABHA status looks goodThis method will help determine if a specific health ID number is new by querying the
isNewAbhacolumn from thet_healthidtable.However, I noticed the order by clause uses
order by 1without specifying a column name. Consider using an explicit column name to avoid potential issues if the table structure changes in the future.- @Query(value = "select isNewAbha from t_healthid where HealthIdNumber=:healthIdNumber order by 1 desc limit 1", nativeQuery = true) + @Query(value = "select isNewAbha from t_healthid where HealthIdNumber=:healthIdNumber order by CreatedDate desc limit 1", nativeQuery = true)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/main/java/com/wipro/fhir/data/healthID/BenHealthIDMapping.java(2 hunks)src/main/java/com/wipro/fhir/repo/healthID/BenHealthIDMappingRepo.java(1 hunks)src/main/java/com/wipro/fhir/service/healthID/HealthIDServiceImpl.java(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyze (java)
🔇 Additional comments (5)
src/main/java/com/wipro/fhir/data/healthID/BenHealthIDMapping.java (2)
33-33: Good use of Lombok to reduce boilerplate codeThe
@Dataannotation is a convenient shortcut that bundles several Lombok annotations (@Getter, @Setter, @tostring, @EqualsAndHashCode, and @requiredargsconstructor), which will generate all the getters and setters automatically.Also applies to: 38-38
78-79: New transient field for tracking new ABHA statusThe
isNewAbhafield is correctly marked as@Transientsince it doesn't need to be persisted to the database and is populated from another table.src/main/java/com/wipro/fhir/service/healthID/HealthIDServiceImpl.java (3)
65-65: Variable naming consistency improvementsGood changes to maintain consistency in variable naming by changing
getBeneficiaryRegId/setBeneficiaryRegIdtogetBeneficiaryRegID/setBeneficiaryRegIDto match the field name in the class.Also applies to: 67-67, 72-72
120-131: Good implementation for enriching health details with ABHA statusThe new code properly enriches each health detail with the ABHA status information before returning the results, which enhances the API's functionality.
120-131:⚠️ Potential issueMethod name inconsistency in enriching health details
The code is using
setNewAbha()in line 126, but the field in theBenHealthIDMappingclass is namedisNewAbha. This naming inconsistency might cause issues.Lombok generates getters and setters with specific naming conventions. For boolean fields, getters are prefixed with "is" and setters with "set". Please update the code to use the correct method name:
- healthDetails.setNewAbha(isNewAbha); + healthDetails.setIsNewAbha(isNewAbha);Additionally, consider adding error handling for potential exceptions when calling
getIsNewAbha(), especially for cases where the health ID number might not exist in the database.Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
src/main/java/com/wipro/fhir/service/common/CommonServiceImpl.java (2)
164-164: Fix typo in log messageThere's a typo in the log message: "dagetPatientListForResourceEligibleys" should be "days".
- logger.info("No of records available to create FHIR in last 2 dagetPatientListForResourceEligibleys : " + pList.size()); + logger.info("No of records available to create FHIR in last 2 days : " + pList.size());
336-342: Remove commented-out code blocksThere are several commented-out code blocks that are no longer needed after the refactoring. Consider removing these to improve code readability and maintainability.
-// } -// if (pcc != null && pcc.getIdentifier() != null) { -// ccList = pcc.getCareContextsList(); -// ccList.add(cc); -// pcc.setCareContextsList(ccList); -// resultSet = patientCareContextsMongoRepo.save(pcc); -//.github/workflows/sast.yml (1)
44-45: Consistent Java Distribution & Trailing Space CleanupThe JDK 17 setup has been updated to use
distribution: 'adopt'which aligns with other workflow files. However, static analysis detects trailing spaces on line 45. It is recommended to remove these trailing spaces to adhere to YAML best practices.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 45-45: trailing spaces
(trailing-spaces)
src/main/environment/common_ci.properties (1)
1-113: JWT Secret Property RemovalThe
jwt.secretconfiguration property has been intentionally removed from this file per our security update. Please ensure that the JWT secret is now provided via a secure mechanism (e.g., environment variables, a dedicated secrets manager, etc.) and that the configuration documentation is updated to reflect this change. If this removal affects any runtime authentication/authorization processes, verify that alternative secure key provisioning is in place.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (20)
.github/workflows/build-on-pull-request.yml(1 hunks).github/workflows/package.yml(2 hunks).github/workflows/sast.yml(1 hunks)pom.xml(4 hunks)src/main/environment/common_ci.properties(1 hunks)src/main/environment/common_dev.properties(0 hunks)src/main/environment/common_example.properties(0 hunks)src/main/environment/common_test.properties(0 hunks)src/main/java/com/wipro/fhir/FhirApiApplication.java(0 hunks)src/main/java/com/wipro/fhir/config/RedisConfig.java(0 hunks)src/main/java/com/wipro/fhir/data/users/User.java(0 hunks)src/main/java/com/wipro/fhir/data/v3/abhaCard/LoginMethod.java(1 hunks)src/main/java/com/wipro/fhir/repo/user/UserLoginRepo.java(0 hunks)src/main/java/com/wipro/fhir/service/common/CommonServiceImpl.java(4 hunks)src/main/java/com/wipro/fhir/service/v3/abha/LoginAbhaV3ServiceImpl.java(1 hunks)src/main/java/com/wipro/fhir/utils/CookieUtil.java(0 hunks)src/main/java/com/wipro/fhir/utils/FilterConfig.java(0 hunks)src/main/java/com/wipro/fhir/utils/JwtAuthenticationUtil.java(0 hunks)src/main/java/com/wipro/fhir/utils/JwtUserIdValidationFilter.java(0 hunks)src/main/java/com/wipro/fhir/utils/JwtUtil.java(0 hunks)
💤 Files with no reviewable changes (12)
- src/main/environment/common_test.properties
- src/main/environment/common_dev.properties
- src/main/environment/common_example.properties
- src/main/java/com/wipro/fhir/repo/user/UserLoginRepo.java
- src/main/java/com/wipro/fhir/config/RedisConfig.java
- src/main/java/com/wipro/fhir/FhirApiApplication.java
- src/main/java/com/wipro/fhir/utils/FilterConfig.java
- src/main/java/com/wipro/fhir/utils/CookieUtil.java
- src/main/java/com/wipro/fhir/utils/JwtAuthenticationUtil.java
- src/main/java/com/wipro/fhir/utils/JwtUserIdValidationFilter.java
- src/main/java/com/wipro/fhir/data/users/User.java
- src/main/java/com/wipro/fhir/utils/JwtUtil.java
🧰 Additional context used
🧠 Learnings (1)
src/main/java/com/wipro/fhir/service/v3/abha/LoginAbhaV3ServiceImpl.java (3)
Learnt from: helenKaryamsetty
PR: PSMRI/FHIR-API#52
File: src/main/java/com/wipro/fhir/service/v3/abha/LoginAbhaV3ServiceImpl.java:84-91
Timestamp: 2025-03-21T08:17:53.052Z
Learning: In the `requestOtpForAbhaLogin` and `verifyAbhaLogin` methods of `LoginAbhaV3ServiceImpl.java`, `loginId` is mandatory, but even if it is null, the process can proceed without input validation.
Learnt from: helenKaryamsetty
PR: PSMRI/FHIR-API#57
File: src/main/java/com/wipro/fhir/service/v3/abha/LoginAbhaV3ServiceImpl.java:242-251
Timestamp: 2025-03-21T08:17:59.398Z
Learning: The ABDM API response for user verification has a fixed structure where "accounts" and "users" arrays are guaranteed to contain data when present, making null/empty checks unnecessary. This is part of the API contract with ABDM.
Learnt from: helenKaryamsetty
PR: PSMRI/FHIR-API#58
File: src/main/java/com/wipro/fhir/service/v3/abha/LoginAbhaV3ServiceImpl.java:0-0
Timestamp: 2025-03-21T08:17:53.052Z
Learning: In the FHIR-API project's ABDM integration, complete responseEntity logging is preferred over just status codes for better debugging capabilities, particularly in the LoginAbhaV3ServiceImpl class when handling ABDM API responses.
🪛 YAMLlint (1.35.1)
.github/workflows/sast.yml
[error] 45-45: trailing spaces
(trailing-spaces)
.github/workflows/package.yml
[error] 38-38: trailing spaces
(trailing-spaces)
🪛 actionlint (1.7.4)
.github/workflows/package.yml
38-38: the runner of "actions/upload-artifact@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🔇 Additional comments (12)
src/main/java/com/wipro/fhir/data/v3/abhaCard/LoginMethod.java (1)
10-10: Field addition aligns with login verification enhancements.The addition of the
loginHintfield complements the existing authentication data structure and enables more specific login verification logic. This field will be used in conjunction with theloginMethodto determine the appropriate authentication flow.src/main/java/com/wipro/fhir/service/v3/abha/LoginAbhaV3ServiceImpl.java (1)
250-251: Improved token verification with dual condition check.The condition has been refined to verify both
loginMethodandloginHintare "MOBILE" before proceeding with profile login verification. This makes the authentication logic more precise and ensures the token verification process is only triggered under the appropriate circumstances.This change aligns with the retrieved learning about ABDM API responses having a fixed structure, where carefully checking conditions before processing the response data is important for proper integration.
src/main/java/com/wipro/fhir/service/common/CommonServiceImpl.java (2)
317-378: Good refactoring of the care context handling logicThe code has been effectively simplified by using a single variable
pccthroughout the method instead of multiple variables. The logic for checking and adding care contexts remains intact while being more maintainable.
324-331: Good practice: Checking for duplicate visit codesThe code properly checks for existing visit codes before adding new ones to the care context list, which prevents duplicate entries. This is a good practice for maintaining data integrity.
.github/workflows/build-on-pull-request.yml (1)
20-20: Java Distribution Update ConfirmedThe JDK setup now specifies
distribution: 'adopt'instead of the previous value. This change appears consistent with similar updates across your workflows. Please make sure that theadoptdistribution meets all your environment requirements..github/workflows/sast.yml (1)
47-47: Updated Maven Build Command with Environment VariableThe build step now includes an environment variable (
-DENV_VAR=test), which can help in setting specific build configurations. Please verify that this change is in line with your intended build/test procedures..github/workflows/package.yml (1)
29-29: Java Distribution Update ConfirmedThe JDK setup now uses
distribution: 'adopt', which is consistent with the changes made in your other workflow files.pom.xml (5)
61-65: Improved Dependency Formatting forlogback-ecs-encoderThe dependency declaration for
logback-ecs-encoderhas been reformatted with consistent indentation and line breaks. This change enhances the readability and maintainability of the POM file.
192-193: Enhanced Comment Formatting for HAPI-FHIR Structures-R4The multi-line comment preceding the
hapi-fhir-structures-r4dependency has been adjusted to include clearer line breaks. This improves clarity on the source URL and keeps the documentation in the POM consistent.
201-202: Improved Comment Clarity for HL7 FHIR UtilitiesThe comment for the
org.hl7.fhir.utilitiesdependency has been reformatted for better readability. Aligning the comment style with other dependency blocks supports overall consistency in documentation.
253-253: Minor Whitespace Cleanup in Plugin ConfigurationThe removal of extraneous whitespace at this location helps maintain a tidy XML structure, which improves visual clarity in the plugin configuration section.
327-337: Consistent Formatting in Resource Merging TaskThe reformatting within the
<echo>and<concat>tasks under themaven-antrun-pluginexecution provides a clearer, more structured layout. This consistency aids in maintaining the resource merging configuration and reduces potential errors during XML editing.
This reverts commit 9f0372c.
|
2 participants
📋 Description
JIRA ID: AMM-1273
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
New Features
Bug Fixes
Refactor
Chores