-
Notifications
You must be signed in to change notification settings - Fork 25
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weβll occasionally send you account related emails.
Already on GitHub? Sign in to your account
API Changes for FLW created ABHA IDs data sync #63
Conversation
WalkthroughThis pull request removes the Changes
Sequence Diagram(s)sequenceDiagram
participant C as Client
participant CR as CreateHealthIdRecord
participant HS as HealthIDServiceImpl
participant HR as HealthIDRepo
C->>+CR: POST /mapHealthIDToBeneficiary (request, Authorization)
CR->>+HS: mapHealthIDToBeneficiary(request, Authorization)
HS->>+HR: Check if health ID exists in t_healthId
HR-->>-HS: Return health ID count
HS-->>-CR: Construct HealthIDResponse or error
CR-->>-C: Return response
C->>+CR: POST /addHealthIdRecord (request, Authorization)
CR->>+HS: addRecordToHealthIdTable(request, Authorization)
HS->>+HR: Check for existing health ID
HR-->>-HS: Return health ID count
HS-->>-CR: Save record and return status
CR-->>-C: Return response
Possibly related PRs
Suggested reviewers
Poem
π Recent review detailsConfiguration used: CodeRabbit UI π Files selected for processing (1)
π§ Files skipped from review as they are similar to previous changes (1)
β° Context from checks skipped due to timeout of 90000ms (1)
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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
π§Ή Nitpick comments (5)
src/main/java/com/wipro/fhir/service/healthID/HealthIDService.java (1)
29-29
: Consider documenting the new interface method more thoroughly.
While the signature is self-explanatory, adding JavaDoc or method-level comments about the expected JSON format, usage context, and concurrency considerations will enhance maintainability and help prevent misuse.src/main/java/com/wipro/fhir/service/healthID/HealthIDServiceImpl.java (1)
109-111
: Avoid overly broad exception handling.
Catching all exceptions underException e
obscures the specific causes of errors. Prefer catching narrower exceptions or rethrowing the original exception to preserve diagnostic information.src/main/java/com/wipro/fhir/controller/healthID/CreateHealthIdRecord.java (2)
37-54
: Improve request validation.
mapHealthIDToBeneficiary
directly accepts a raw JSON string, leaving validation to deeper service layers. Consider binding to a typed DTO or adding validation checks to guard against malformed JSON or incomplete fields.
59-76
: Enhance error handling and logging.
While you log the encountered exception, consider usinglogger.error("Error mapping health ID", e)
or a similar pattern to include stack trace details. Also, ensure that error codes follow a consistent scheme across the application to facilitate debugging.src/main/java/com/wipro/fhir/data/healthID/HealthIDRequestAadhar.java (1)
41-42
: Consider using a date-specific type for the dob field.The
dob
field is currently defined as a String, which might not be the most type-safe approach for handling dates. Consider using Java's date types likeLocalDate
orDate
to ensure proper date validation and formatting.String password; String profilePhoto; Boolean isNew; - String dob; + LocalDate dob;You would also need to add the import:
import java.time.LocalDate;
π Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
π Files selected for processing (5)
src/main/java/com/wipro/fhir/controller/healthID/CreateHealthIDWithMobileOTP.java
(0 hunks)src/main/java/com/wipro/fhir/controller/healthID/CreateHealthIdRecord.java
(1 hunks)src/main/java/com/wipro/fhir/data/healthID/HealthIDRequestAadhar.java
(2 hunks)src/main/java/com/wipro/fhir/service/healthID/HealthIDService.java
(1 hunks)src/main/java/com/wipro/fhir/service/healthID/HealthIDServiceImpl.java
(5 hunks)
π€ Files with no reviewable changes (1)
- src/main/java/com/wipro/fhir/controller/healthID/CreateHealthIDWithMobileOTP.java
β° Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyze (java)
π Additional comments (5)
src/main/java/com/wipro/fhir/service/healthID/HealthIDServiceImpl.java (4)
60-75
: Validate request fields early.
WithinmapHealthIDToBeneficiary
, the code references several request fields (e.g.,beneficiaryRegId
,beneficiaryID
,healthIdNumber
) but lacks explicit field-level validation for JSON segments likeABHAProfile
. Ensure the request includes all required fields to avoid future NullPointerExceptions.
76-107
: Address potential concurrency issue when creating new ABHA records.
The checkif (healthIdCount < 1)
may lead to race conditions if multiple simultaneous requests arrive with the same ABHA details. Consider adding a unique constraint at the database layer or employing a locking strategy to guarantee data integrity.
131-132
: Concurrency concern repeats here.
The same race condition identified inmapHealthIDToBeneficiary
applies to theif (healthIdCount < 1)
check. A unique DB constraint or locking mechanism can help ensure correctness.
163-164
: Broad catch block repeats here.
This exception-handling pattern is the same as above. Narrowing the catch scope makes debugging and error handling more precise.src/main/java/com/wipro/fhir/data/healthID/HealthIDRequestAadhar.java (1)
24-26
: Good use of Lombok to reduce boilerplate code.Using Lombok's
@Data
annotation is a great improvement that eliminates the need for manually writing getters, setters, equals, hashCode, and toString methods. This makes the code more maintainable and concise.
|
||
@Override | ||
public String addRecordToHealthIdTable(String request) throws FHIRException { | ||
JsonObject jsonRequest = JsonParser.parseString(request).getAsJsonObject(); | ||
JsonObject abhaProfileJson = jsonRequest.getAsJsonObject("ABHAProfile"); | ||
HealthIDResponse healthID = InputMapper.gson().fromJson(abhaProfileJson, HealthIDResponse.class); | ||
String res = null; | ||
try { | ||
Integer healthIdCount = healthIDRepo.getCountOfHealthIdNumber(healthID.getHealthIdNumber()); | ||
if(healthIdCount < 1) { | ||
healthID.setHealthIdNumber(abhaProfileJson.get("ABHANumber").getAsString()); | ||
JsonArray phrAddressArray = abhaProfileJson.getAsJsonArray("phrAddress"); | ||
StringBuilder abhaAddressBuilder = new StringBuilder(); | ||
|
||
for (int i = 0; i < phrAddressArray.size(); i++) { | ||
abhaAddressBuilder.append(phrAddressArray.get(i).getAsString()); | ||
if (i < phrAddressArray.size() - 1) { | ||
abhaAddressBuilder.append(", "); | ||
} | ||
} | ||
healthID.setHealthId(abhaAddressBuilder.toString()); | ||
healthID.setName( | ||
abhaProfileJson.get("firstName").getAsString() + " " + abhaProfileJson.get("middleName").getAsString() + " " + abhaProfileJson.get("lastName").getAsString()); | ||
SimpleDateFormat simpleDateFormat = new SimpleDateFormat("dd-MM-yyyy"); | ||
Date date = simpleDateFormat.parse(abhaProfileJson.get("dob").getAsString()); | ||
SimpleDateFormat year = new SimpleDateFormat("yyyy"); | ||
SimpleDateFormat month = new SimpleDateFormat("MM"); | ||
SimpleDateFormat day = new SimpleDateFormat("dd"); | ||
healthID.setYearOfBirth(year.format(date)); | ||
healthID.setMonthOfBirth(month.format(date)); | ||
healthID.setDayOfBirth(day.format(date)); | ||
healthID.setCreatedBy(jsonRequest.get("createdBy").getAsString()); | ||
healthID.setProviderServiceMapID(jsonRequest.get("providerServiceMapId").getAsInt()); | ||
healthID.setIsNewAbha(jsonRequest.get("isNew").getAsBoolean()); | ||
healthIDRepo.save(healthID); | ||
res = "Data Saved Successfully"; | ||
} else { | ||
res = "Data already exists"; | ||
} | ||
} catch (Exception e) { | ||
throw new FHIRException("Error in saving data"); | ||
} | ||
return res; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
π οΈ Refactor suggestion
Refactor to reduce duplicate JSON parsing logic.
This method mirrors much of the JSON parsing and entity population performed in mapHealthIDToBeneficiary
. Consider extracting a private utility method to parse and populate HealthIDResponse
so both methods can reuse it, reducing maintenance overhead.
|
src/main/java/com/wipro/fhir/controller/healthID/CreateHealthIdRecord.java
Dismissed
Show dismissed
Hide dismissed
src/main/java/com/wipro/fhir/controller/healthID/CreateHealthIdRecord.java
Dismissed
Show dismissed
Hide dismissed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks ok
π Description
JIRA ID: AMM-1262
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