Work on asha update profile and get profile , lmp date #10
Conversation
# Conflicts: # src/main/resources/application.properties
# Conflicts: # src/main/java/com/iemr/flw/controller/AshaProfileController.java
MicroBirthPlan Otp Beneficiary Asha worker profile lmp date
WalkthroughA comprehensive update was applied across configuration files, controllers, domain models, DTOs, repositories, services, and utilities. The environment is now fixed to "local" with updated database, Redis, and SMS settings. Several new REST controllers and endpoints were introduced for handling ASHA profiles, OTP requests, micro birth plans, and maternal health visits (including file uploads). Domain models were extended with new fields, and multiple service interfaces and implementations were added to support these features. Minor adjustments were also made to logging and utility functions. Changes
Sequence Diagram(s)sequenceDiagram
participant C as Client
participant OTPCtrl as BeneficiaryOTPGatewayController
participant OTPService as OTPHandlerServiceImpl
participant SMS as SMS Gateway
C->>OTPCtrl: Request sendOTP(phoneNumber)
OTPCtrl->>OTPService: sendOTP(phoneNumber)
OTPService->>SMS: Trigger SMS with OTP
SMS-->>OTPService: SMS response
OTPService-->>OTPCtrl: Return success message & OTP
OTPCtrl-->>C: Deliver OTP response
sequenceDiagram
participant C as Client
participant MHC as MaternalHealthController
participant FSS as FileStorageService
participant MHS as MaternalHealthService
C->>MHC: Submit ANC visit data with file upload
MHC->>FSS: Call storeFile(file)
FSS-->>MHC: Return stored file path
MHC->>MHS: Save ANC visit details with file path
MHS-->>MHC: Confirm data saved
MHC-->>C: Respond with OK status
Poem
Tip CodeRabbit's docstrings feature is now available as part of our Pro Plan! Simply use the command ✨ Finishing Touches
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: 34
🔭 Outside diff range comments (1)
src/main/java/com/iemr/flw/controller/UserController.java (1)
20-20:⚠️ Potential issueFix logger class reference.
The logger is incorrectly using
CoupleController.classinstead ofUserController.class.Apply this diff:
- private final Logger logger = LoggerFactory.getLogger(CoupleController.class); + private final Logger logger = LoggerFactory.getLogger(UserController.class);
🧹 Nitpick comments (20)
src/main/java/com/iemr/flw/domain/iemr/EligibleCoupleTracking.java (1)
53-54: LGTM! Consider adding validation for the LMP date.The field addition follows the existing patterns and naming conventions. Consider adding validation to ensure the LMP date is not set in the future.
@Column(name = "lmp_date") @PrePersist @PreUpdate private void validateLmpDate() { if (lmpDate != null && lmpDate.after(new Timestamp(System.currentTimeMillis()))) { throw new IllegalArgumentException("LMP date cannot be in the future"); } } private Timestamp lmpDate;src/main/java/com/iemr/flw/domain/iemr/EligibleCoupleRegister.java (2)
161-163: LGTM! Consider adding validation for the LMP date.The field addition follows the existing patterns and naming conventions. Consider adding validation to ensure the LMP date is not set in the future and is consistent with other pregnancy-related dates.
@Column(name = "lmp_date") @PrePersist @PreUpdate private void validateLmpDate() { if (lmpDate != null) { if (lmpDate.after(new Timestamp(System.currentTimeMillis()))) { throw new IllegalArgumentException("LMP date cannot be in the future"); } // Validate against first child's birth date if exists if (dob1 != null && !lmpDate.after(dob1)) { throw new IllegalArgumentException("LMP date must be after first child's birth date"); } } } private Timestamp lmpDate;
164-167: Remove extra blank lines at the end of the file.Maintain consistent file formatting by removing the extra blank lines.
src/main/java/com/iemr/flw/controller/MicroBirthPlanController.java (1)
26-29: Standardize endpoint naming and enhance documentation.The endpoint names are inconsistent (downSync vs getAllMicroBirthPlansById). Also, enhance the API documentation with detailed operation descriptions.
- @RequestMapping(value = "downSync",method = RequestMethod.GET) + @Operation(summary = "Get all micro birth plans", description = "Retrieves all micro birth plans from the system") + @GetMapping("/all") public ResponseEntity<List<MicroBirthPlan>> getAllMicroBirthPlans() { return ResponseEntity.ok(service.getAllMicroBirthPlans()); } - @RequestMapping(value = "getAllMicroBirthPlansBy{id}",method = RequestMethod.GET) + @Operation(summary = "Get micro birth plan by ID", description = "Retrieves a specific micro birth plan using its ID") + @GetMapping("/{id}") public ResponseEntity<MicroBirthPlan> getMicroBirthPlanById(@PathVariable Long id) {Also applies to: 31-36
src/main/java/com/iemr/flw/domain/iemr/MicroBirthPlan.java (2)
8-9: Replace @DaTa with more specific Lombok annotations.@DaTa can lead to performance issues due to toString() on large objects and potential equals/hashCode problems. Consider using more specific annotations.
-@Data +@Getter +@Setter +@ToString(exclude = {"bloodDonors", "birthCompanion", "childCaretaker"}) +@EqualsAndHashCode(of = "id")
66-67: Add audit fields for better tracking.Include created and updated timestamp fields for better data tracking.
@Column(name = "is_synced") private Boolean isSynced; + + @Column(name = "created_at") + @CreationTimestamp + private LocalDateTime createdAt; + + @Column(name = "updated_at") + @UpdateTimestamp + private LocalDateTime updatedAt;src/main/java/com/iemr/flw/service/MicroBirthPlanService.java (1)
52-54: Add safe deletion check and logging.The delete operation should verify existence before deletion and include logging.
+ @Transactional public void deleteMicroBirthPlan(Long id) { + log.info("Attempting to delete MicroBirthPlan with id: {}", id); + if (!repository.existsById(id)) { + throw new ResourceNotFoundException("MicroBirthPlan not found with id: " + id); + } repository.deleteById(id); + log.info("Successfully deleted MicroBirthPlan with id: {}", id); }src/main/java/com/iemr/flw/service/impl/OTPHandlerServiceImpl.java (3)
103-116: Consider transaction boundaries and concurrency checks.
When marking OTP entries as verified and expired, ensure a proper transactional context so that concurrent verifications for the same OTP don't lead to inconsistent states or race conditions.
146-157: Add salt or pepper for stronger OTP hashing.
Relying purely on a SHA-256 hash without additional salt or pepper leaves the OTP vulnerable to rainbow-table attacks if the hashes are exposed. Consider introducing a salt or shared secret.
159-209: Refactor the lengthy SMS gateway switch logic.
The nested switch case can be extracted to a map or dedicated error-handling method for easier maintenance and readability.src/main/java/com/iemr/flw/controller/BeneficiaryOTPGatewayController.java (1)
71-91: Add rate limiting for resend OTP endpoint.The resend OTP endpoint should be rate-limited to prevent abuse.
Consider implementing a rate limiter using Redis (since it's already configured) to prevent multiple OTP requests from the same phone number within a short time period.
src/main/resources/application.properties (2)
51-57: Obsolete Commented Configurations.
There are commented-out lines for FHIR and TM configuration (lines 51–57) that duplicate the active settings. Removing these commented lines—if they are no longer needed—could help reduce clutter and avoid potential confusion in the future.
78-84: SMS Service Configuration & Formatting Improvements.
The new SMS-related properties (lines 78–84) integrate with an external SMS gateway. Please consider the following adjustments for consistency and correct parsing:
- Remove extraneous spaces around the equals signs.
- Eliminate the unnecessary quotes in
sms-sender-numberto avoid unintended characters in the value.For example, apply the following diff:
- sendSMSUrl = v1/send-sms + sendSMSUrl=v1/send-sms - sms-sender-number ="9560618681" + sms-sender-number=9560618681 - send-message-url= v1/send-sms + send-message-url=v1/send-sms - sms-gateway-url= https://iqsms.airtel.in/api + sms-gateway-url=https://iqsms.airtel.in/apiAlso, verify that exposing SMS credentials in this file is acceptable for the local configuration and that no sensitive production data is inadvertently committed.
src/main/java/com/iemr/flw/service/EmployeeMasterInter.java (1)
7-9: Consider adding documentation and null handling.The interface looks good but could benefit from:
- JavaDoc documentation describing the method's purpose and parameters
@Nullableannotation to indicate if null returns are possibleAdd documentation and null handling:
@Service public interface EmployeeMasterInter { + /** + * Retrieves user details for the given user ID + * @param userID The ID of the user to retrieve + * @return The user details or null if not found + */ + @Nullable public M_User getUserDetails(Integer userID);src/main/java/com/iemr/flw/service/AshaProfileService.java (1)
7-10: Add validation and documentation for the service interface.The interface looks good but could benefit from:
- JavaDoc documentation for methods
@Validannotation for input validation@NotNullfor required parametersAdd documentation and validation:
@Service public interface AshaProfileService { + /** + * Saves or updates ASHA worker profile data + * @param editdata The ASHA worker data to save/update + * @return The saved/updated ASHA worker profile + */ + AshaWorker saveEditData(@Valid @NotNull AshaWorker editdata); + + /** + * Retrieves ASHA worker profile by employee ID + * @param employeeId The employee ID to look up + * @return The ASHA worker profile or null if not found + */ + @Nullable AshaWorker getProfileData(@NotNull Integer employeeId);src/main/java/com/iemr/flw/repo/iemr/EmployeeMasterRepo.java (1)
8-10: Optimize query performance with Spring annotations.The repository interface looks good but could benefit from query optimization:
Add performance optimizations:
@Repository public interface EmployeeMasterRepo extends JpaRepository<M_User,Integer> { + /** + * Find user by their unique user ID + * @param userID The user ID to look up + * @return The user or null if not found + */ + @QueryHints(value = { @QueryHint(name = "org.hibernate.cacheable", value = "true") }) M_User findByUserID(Integer userID);src/main/java/com/iemr/flw/repo/iemr/AshaProfileRepo.java (1)
7-10: Optimize query performance and add index hint.The repository interface looks good but could benefit from:
- Query optimization annotations
- Index hint for employeeId field
Add performance optimizations:
@Repository public interface AshaProfileRepo extends JpaRepository<AshaWorker,Integer> { + /** + * Find ASHA worker profile by employee ID + * @param employeeId The employee ID to look up + * @return The ASHA worker profile or null if not found + */ + @QueryHints(value = { @QueryHint(name = "org.hibernate.cacheable", value = "true") }) AshaWorker findByEmployeeId(Integer employeeId); } +// Add to AshaWorker.java: +@Entity +public class AshaWorker { + @Column(name = "employeeId", unique = true) + @Index(name = "idx_employee_id") + private Integer employeeId; +}src/main/java/com/iemr/flw/service/impl/EmployeeMasterImpl.java (1)
11-12: Consider using constructor injection instead of field injection.Replace field-level @Autowired with constructor injection for better testability and explicit dependency declaration.
- @Autowired - EmployeeMasterRepo employeeMasterRepo; + private final EmployeeMasterRepo employeeMasterRepo; + + public EmployeeMasterImpl(EmployeeMasterRepo employeeMasterRepo) { + this.employeeMasterRepo = employeeMasterRepo; + }src/main/java/com/iemr/flw/domain/iemr/AshaWorker.java (1)
98-99: Fix naming convention violation.The field name 'ProviderServiceMapID' violates Java naming conventions. Use camelCase for field names.
- private Integer ProviderServiceMapID; + private Integer providerServiceMapId;src/main/java/com/iemr/flw/service/impl/AshaProfileImpl.java (1)
15-22: Add constructor-based dependency injection and class documentation.Consider these improvements for better maintainability and reliability:
- Use constructor-based dependency injection instead of field injection
- Add class-level documentation explaining the purpose and responsibilities
@Service +/** + * Service implementation for managing ASHA worker profiles. + * Handles profile creation, updates, and retrieval operations. + */ public class AshaProfileImpl implements AshaProfileService { - @Autowired - AshaProfileRepo ashaProfileRepo; - @Autowired - EmployeeMasterInter employeeMasterInter; + private final AshaProfileRepo ashaProfileRepo; + private final EmployeeMasterInter employeeMasterInter; + private final Logger logger = LoggerFactory.getLogger(AshaProfileImpl.class); - private final Logger logger = LoggerFactory.getLogger(AshaProfileImpl.class); + public AshaProfileImpl(AshaProfileRepo ashaProfileRepo, + EmployeeMasterInter employeeMasterInter) { + this.ashaProfileRepo = Objects.requireNonNull(ashaProfileRepo, "ashaProfileRepo must not be null"); + this.employeeMasterInter = Objects.requireNonNull(employeeMasterInter, "employeeMasterInter must not be null"); + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (34)
pom.xml(1 hunks)src/main/environment/common_local.properties(1 hunks)src/main/java/com/iemr/flw/controller/AshaProfileController.java(1 hunks)src/main/java/com/iemr/flw/controller/BeneficiaryOTPGatewayController.java(1 hunks)src/main/java/com/iemr/flw/controller/MaternalHealthController.java(3 hunks)src/main/java/com/iemr/flw/controller/MicroBirthPlanController.java(1 hunks)src/main/java/com/iemr/flw/controller/UserController.java(1 hunks)src/main/java/com/iemr/flw/domain/iemr/AshaWorker.java(1 hunks)src/main/java/com/iemr/flw/domain/iemr/EligibleCoupleRegister.java(1 hunks)src/main/java/com/iemr/flw/domain/iemr/EligibleCoupleTracking.java(1 hunks)src/main/java/com/iemr/flw/domain/iemr/M_User.java(1 hunks)src/main/java/com/iemr/flw/domain/iemr/MicroBirthPlan.java(1 hunks)src/main/java/com/iemr/flw/domain/iemr/OtpBeneficiary.java(1 hunks)src/main/java/com/iemr/flw/dto/iemr/ANCVisitDTO.java(2 hunks)src/main/java/com/iemr/flw/dto/iemr/EligibleCoupleDTO.java(1 hunks)src/main/java/com/iemr/flw/dto/iemr/OTPRequestParsor.java(1 hunks)src/main/java/com/iemr/flw/dto/iemr/OtpRequestDTO.java(1 hunks)src/main/java/com/iemr/flw/repo/iemr/AshaProfileRepo.java(1 hunks)src/main/java/com/iemr/flw/repo/iemr/EmployeeMasterRepo.java(1 hunks)src/main/java/com/iemr/flw/repo/iemr/MicroBirthPlanRepository.java(1 hunks)src/main/java/com/iemr/flw/repo/iemr/OtpBeneficiaryRepository.java(1 hunks)src/main/java/com/iemr/flw/repo/iemr/UserServiceRoleRepo.java(1 hunks)src/main/java/com/iemr/flw/service/AshaProfileService.java(1 hunks)src/main/java/com/iemr/flw/service/EmployeeMasterInter.java(1 hunks)src/main/java/com/iemr/flw/service/FileStorageService.java(1 hunks)src/main/java/com/iemr/flw/service/MicroBirthPlanService.java(1 hunks)src/main/java/com/iemr/flw/service/OTPHandler.java(1 hunks)src/main/java/com/iemr/flw/service/impl/AshaProfileImpl.java(1 hunks)src/main/java/com/iemr/flw/service/impl/EmployeeMasterImpl.java(1 hunks)src/main/java/com/iemr/flw/service/impl/OTPHandlerServiceImpl.java(1 hunks)src/main/java/com/iemr/flw/utils/http/HTTPRequestInterceptor.java(1 hunks)src/main/java/com/iemr/flw/utils/http/HttpUtils.java(1 hunks)src/main/java/com/iemr/flw/utils/redis/RedisStorage.java(2 hunks)src/main/resources/application.properties(2 hunks)
✅ Files skipped from review due to trivial changes (6)
- src/main/java/com/iemr/flw/dto/iemr/ANCVisitDTO.java
- src/main/java/com/iemr/flw/dto/iemr/OTPRequestParsor.java
- src/main/java/com/iemr/flw/dto/iemr/OtpRequestDTO.java
- src/main/java/com/iemr/flw/repo/iemr/MicroBirthPlanRepository.java
- src/main/java/com/iemr/flw/repo/iemr/UserServiceRoleRepo.java
- src/main/java/com/iemr/flw/utils/redis/RedisStorage.java
🔇 Additional comments (8)
src/main/java/com/iemr/flw/dto/iemr/EligibleCoupleDTO.java (1)
109-109: LGTM! Field addition is consistent with the entity.The DTO field matches the entity class, maintaining consistency in the data model.
src/main/java/com/iemr/flw/repo/iemr/OtpBeneficiaryRepository.java (1)
10-11: Looks good!
The custom finder methodfindByPhoneNumberAndOtpis concise and follows Spring Data conventions. No immediate issues found.src/main/resources/application.properties (5)
4-4: FHIR API Endpoint Configuration Added.
The new propertyfhir-urlis set tohttps://uatamrit.piramalswasthya.org/fhirapi-v1.0. Verify that this URL is correct and consider whether a more descriptive or namespaced key (e.g.,application.fhir.url) might improve clarity and consistency with other configuration keys.
6-7: TM API Endpoint Configuration Added.
The addition oftm-url=https://uatamrit.piramalswasthya.org/tmapi-v1.0appropriately mirrors the FHIR configuration. Confirm that external integrations use this property as intended.
59-64: Primary Database Connection Details.
The properties for the primary database (lines 59–64) are now explicitly defined, usingjdbc:mysql://localhost:3306/db_iemrand setting the username and password. Ensure that these credentials are intended only for a local or non-production environment. In production scenarios, consider externalizing or securing sensitive credentials.
68-73: Secondary Database Connection Setup.
The secondary database configuration (lines 68–73) mirrors the primary configuration with appropriate connection details. As with the primary DB, verify that these hardcoded credentials are acceptable in this context and consider secure alternatives for deployment outside of development.
75-76: Enabling API Documentation.
The addition ofspringdoc.api-docs.enabled=trueandspringdoc.swagger-ui.enabled=truefacilitates API exploration and testing through Swagger. This is a valuable enhancement for both developer experience and client integrations.pom.xml (1)
30-30: Hardcoding the Environment to "local".
The<environment>local</environment>property (line 30) forces the use of local configuration properties (src/main/environment/common_local.properties). While this setup benefits local testing by providing concrete values, ensure that deployments to production remain flexible by possibly switching to an externalized environment variable or profile mechanism when needed.
| @Autowired | ||
| private MicroBirthPlanService service; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Replace field injection with constructor injection.
Field injection using @Autowired is discouraged as it makes the class harder to test and hides dependencies.
- @Autowired
- private MicroBirthPlanService service;
+ private final MicroBirthPlanService service;
+
+ public MicroBirthPlanController(MicroBirthPlanService service) {
+ this.service = service;
+ }📝 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.
| @Autowired | |
| private MicroBirthPlanService service; | |
| private final MicroBirthPlanService service; | |
| public MicroBirthPlanController(MicroBirthPlanService service) { | |
| this.service = service; | |
| } |
| @RequestMapping(value = "save",method = RequestMethod.POST) | ||
| public ResponseEntity<MicroBirthPlan> createMicroBirthPlan(@RequestBody MicroBirthPlan birthPlan) { | ||
| return ResponseEntity.ok(service.createMicroBirthPlan(birthPlan)); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add input validation and proper error handling.
The endpoint should validate the input and handle potential errors gracefully.
@RequestMapping(value = "save",method = RequestMethod.POST)
- public ResponseEntity<MicroBirthPlan> createMicroBirthPlan(@RequestBody MicroBirthPlan birthPlan) {
- return ResponseEntity.ok(service.createMicroBirthPlan(birthPlan));
+ public ResponseEntity<MicroBirthPlan> createMicroBirthPlan(@Valid @RequestBody MicroBirthPlan birthPlan) {
+ try {
+ MicroBirthPlan created = service.createMicroBirthPlan(birthPlan);
+ return ResponseEntity.ok(created);
+ } catch (Exception e) {
+ return ResponseEntity.badRequest().build();
+ }
}📝 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.
| @RequestMapping(value = "save",method = RequestMethod.POST) | |
| public ResponseEntity<MicroBirthPlan> createMicroBirthPlan(@RequestBody MicroBirthPlan birthPlan) { | |
| return ResponseEntity.ok(service.createMicroBirthPlan(birthPlan)); | |
| } | |
| @RequestMapping(value = "save",method = RequestMethod.POST) | |
| public ResponseEntity<MicroBirthPlan> createMicroBirthPlan(@Valid @RequestBody MicroBirthPlan birthPlan) { | |
| try { | |
| MicroBirthPlan created = service.createMicroBirthPlan(birthPlan); | |
| return ResponseEntity.ok(created); | |
| } catch (Exception e) { | |
| return ResponseEntity.badRequest().build(); | |
| } | |
| } |
| import java.util.List; | ||
|
|
||
| @RestController | ||
| @RequestMapping(value = "/micro-birthPlan",headers = "Authorization", produces = "application/json") |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider removing 'headers = "Authorization"' from @RequestMapping.
Instead of enforcing Authorization header at the mapping level, consider using Spring Security or an interceptor for authentication/authorization.
-@RequestMapping(value = "/micro-birthPlan",headers = "Authorization", produces = "application/json")
+@RequestMapping(value = "/micro-birthPlan", produces = "application/json")📝 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.
| @RequestMapping(value = "/micro-birthPlan",headers = "Authorization", produces = "application/json") | |
| @RequestMapping(value = "/micro-birthPlan", produces = "application/json") |
| @Column(name = "contact_no_1", length = 10) | ||
| private String contactNo1; // Auto-filled, must be 10 digits, starts with 6-9 | ||
|
|
||
| @Column(name = "contact_no_2", length = 10) | ||
| private String contactNo2; // Optional, 10 digits, starts with 6-9 |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add validation constraints for contact numbers.
The contact numbers require specific validation (10 digits, starts with 6-9) as per comments.
@Column(name = "contact_no_1", length = 10)
+ @Pattern(regexp = "^[6-9]\\d{9}$", message = "Contact number must be 10 digits and start with 6-9")
private String contactNo1;
@Column(name = "contact_no_2", length = 10)
+ @Pattern(regexp = "^[6-9]\\d{9}$", message = "Contact number must be 10 digits and start with 6-9")
private String contactNo2;📝 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.
| @Column(name = "contact_no_1", length = 10) | |
| private String contactNo1; // Auto-filled, must be 10 digits, starts with 6-9 | |
| @Column(name = "contact_no_2", length = 10) | |
| private String contactNo2; // Optional, 10 digits, starts with 6-9 | |
| @Column(name = "contact_no_1", length = 10) | |
| @Pattern(regexp = "^[6-9]\\d{9}$", message = "Contact number must be 10 digits and start with 6-9") | |
| private String contactNo1; // Auto-filled, must be 10 digits, starts with 6-9 | |
| @Column(name = "contact_no_2", length = 10) | |
| @Pattern(regexp = "^[6-9]\\d{9}$", message = "Contact number must be 10 digits and start with 6-9") | |
| private String contactNo2; // Optional, 10 digits, starts with 6-9 |
| @Autowired | ||
| private MicroBirthPlanRepository repository; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Replace field injection with constructor injection.
Field injection using @Autowired is discouraged. Use constructor injection instead.
- @Autowired
- private MicroBirthPlanRepository repository;
+ private final MicroBirthPlanRepository repository;
+
+ public MicroBirthPlanService(MicroBirthPlanRepository repository) {
+ this.repository = repository;
+ }📝 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.
| @Autowired | |
| private MicroBirthPlanRepository repository; | |
| private final MicroBirthPlanRepository repository; | |
| public MicroBirthPlanService(MicroBirthPlanRepository repository) { | |
| this.repository = repository; | |
| } |
| public M_User() { | ||
| // TODO Auto-generated constructor stub | ||
| } | ||
|
|
||
| public M_User(Integer userID, String userName) { | ||
| // TODO Auto-generated constructor stub | ||
| this.userID=userID; | ||
| this.userName=userName; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Implement proper constructors.
Remove TODO comments and implement proper constructors with validation.
public M_User() {
- // TODO Auto-generated constructor stub
}
public M_User(Integer userID, String userName) {
- // TODO Auto-generated constructor stub
+ if (userID == null) {
+ throw new IllegalArgumentException("UserID cannot be null");
+ }
+ if (userName == null || userName.trim().isEmpty()) {
+ throw new IllegalArgumentException("UserName cannot be null or empty");
+ }
this.userID = userID;
this.userName = userName;
}📝 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.
| public M_User() { | |
| // TODO Auto-generated constructor stub | |
| } | |
| public M_User(Integer userID, String userName) { | |
| // TODO Auto-generated constructor stub | |
| this.userID=userID; | |
| this.userName=userName; | |
| } | |
| public M_User() { | |
| } | |
| public M_User(Integer userID, String userName) { | |
| if (userID == null) { | |
| throw new IllegalArgumentException("UserID cannot be null"); | |
| } | |
| if (userName == null || userName.trim().isEmpty()) { | |
| throw new IllegalArgumentException("UserName cannot be null or empty"); | |
| } | |
| this.userID = userID; | |
| this.userName = userName; | |
| } |
| @Override | ||
| public AshaWorker saveEditData(AshaWorker ashaWorker) { | ||
| AshaWorker ashaWorker1; | ||
| if (ashaWorker.getId() != null) { | ||
| ashaWorker1 = ashaProfileRepo.saveAndFlush(updateProfile(ashaWorker)); | ||
|
|
||
| } else { | ||
| ashaWorker1 = ashaProfileRepo.saveAndFlush(ashaWorker); | ||
|
|
||
| } | ||
| System.out.println("ashaWorker->>>" + ashaWorker1.toString()); | ||
|
|
||
| return ashaWorker1; | ||
|
|
||
|
|
||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve error handling and logging in saveEditData method.
The method needs several improvements for robustness:
- Replace
System.out.printlnwith proper logging - Add transaction management
- Add error handling
- Simplify the code structure
+ @Transactional
@Override
public AshaWorker saveEditData(AshaWorker ashaWorker) {
- AshaWorker ashaWorker1;
- if (ashaWorker.getId() != null) {
- ashaWorker1 = ashaProfileRepo.saveAndFlush(updateProfile(ashaWorker));
-
- } else {
- ashaWorker1 = ashaProfileRepo.saveAndFlush(ashaWorker);
-
- }
- System.out.println("ashaWorker->>>" + ashaWorker1.toString());
-
- return ashaWorker1;
-
-
+ try {
+ Objects.requireNonNull(ashaWorker, "ashaWorker must not be null");
+ AshaWorker savedWorker = ashaWorker.getId() != null
+ ? ashaProfileRepo.saveAndFlush(updateProfile(ashaWorker))
+ : ashaProfileRepo.saveAndFlush(ashaWorker);
+ logger.info("ASHA worker profile saved successfully: {}", savedWorker);
+ return savedWorker;
+ } catch (Exception e) {
+ logger.error("Error saving ASHA worker profile: {}", e.getMessage(), e);
+ throw new RuntimeException("Failed to save ASHA worker profile", e);
+ }
}📝 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.
| @Override | |
| public AshaWorker saveEditData(AshaWorker ashaWorker) { | |
| AshaWorker ashaWorker1; | |
| if (ashaWorker.getId() != null) { | |
| ashaWorker1 = ashaProfileRepo.saveAndFlush(updateProfile(ashaWorker)); | |
| } else { | |
| ashaWorker1 = ashaProfileRepo.saveAndFlush(ashaWorker); | |
| } | |
| System.out.println("ashaWorker->>>" + ashaWorker1.toString()); | |
| return ashaWorker1; | |
| } | |
| @Transactional | |
| @Override | |
| public AshaWorker saveEditData(AshaWorker ashaWorker) { | |
| try { | |
| Objects.requireNonNull(ashaWorker, "ashaWorker must not be null"); | |
| AshaWorker savedWorker = ashaWorker.getId() != null | |
| ? ashaProfileRepo.saveAndFlush(updateProfile(ashaWorker)) | |
| : ashaProfileRepo.saveAndFlush(ashaWorker); | |
| logger.info("ASHA worker profile saved successfully: {}", savedWorker); | |
| return savedWorker; | |
| } catch (Exception e) { | |
| logger.error("Error saving ASHA worker profile: {}", e.getMessage(), e); | |
| throw new RuntimeException("Failed to save ASHA worker profile", e); | |
| } | |
| } |
| private AshaWorker getDetails(Integer userID) { | ||
| AshaWorker ashaWorker = new AshaWorker(); | ||
| M_User m_user = employeeMasterInter.getUserDetails(userID); | ||
|
|
||
| ashaWorker.setEmployeeId(m_user.getUserID()); | ||
| ashaWorker.setEmployeeId(m_user.getUserID()); | ||
| ashaWorker.setDob(m_user.getDOB()); | ||
| ashaWorker.setDateOfJoining(m_user.getDOJ()); | ||
| ashaWorker.setName(m_user.getFirstName() + " " + m_user.getLastName()); | ||
| ashaWorker.setMobileNumber(m_user.getContactNo()); | ||
| ashaWorker.setAlternateMobileNumber(m_user.getEmergencyContactNo()); | ||
| ashaWorker.setProviderServiceMapID(m_user.getServiceProviderID()); | ||
| return ashaWorker; | ||
| } |
There was a problem hiding this comment.
Fix duplicate code and add error handling in getDetails method.
The method has a duplicate setEmployeeId call and lacks proper error handling:
private AshaWorker getDetails(Integer userID) {
- AshaWorker ashaWorker = new AshaWorker();
- M_User m_user = employeeMasterInter.getUserDetails(userID);
-
- ashaWorker.setEmployeeId(m_user.getUserID());
- ashaWorker.setEmployeeId(m_user.getUserID());
- ashaWorker.setDob(m_user.getDOB());
- ashaWorker.setDateOfJoining(m_user.getDOJ());
- ashaWorker.setName(m_user.getFirstName() + " " + m_user.getLastName());
- ashaWorker.setMobileNumber(m_user.getContactNo());
- ashaWorker.setAlternateMobileNumber(m_user.getEmergencyContactNo());
- ashaWorker.setProviderServiceMapID(m_user.getServiceProviderID());
- return ashaWorker;
+ try {
+ M_User m_user = Objects.requireNonNull(
+ employeeMasterInter.getUserDetails(userID),
+ "User details not found for ID: " + userID
+ );
+
+ AshaWorker ashaWorker = new AshaWorker();
+ ashaWorker.setEmployeeId(m_user.getUserID());
+ ashaWorker.setDob(m_user.getDOB());
+ ashaWorker.setDateOfJoining(m_user.getDOJ());
+ ashaWorker.setName(String.format("%s %s",
+ Objects.toString(m_user.getFirstName(), ""),
+ Objects.toString(m_user.getLastName(), "")).trim());
+ ashaWorker.setMobileNumber(m_user.getContactNo());
+ ashaWorker.setAlternateMobileNumber(m_user.getEmergencyContactNo());
+ ashaWorker.setProviderServiceMapID(m_user.getServiceProviderID());
+ return ashaWorker;
+ } catch (Exception e) {
+ logger.error("Error creating ASHA worker profile from user details for ID {}: {}", userID, e.getMessage(), e);
+ throw new RuntimeException("Failed to create ASHA worker profile from user details", e);
+ }
}📝 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.
| private AshaWorker getDetails(Integer userID) { | |
| AshaWorker ashaWorker = new AshaWorker(); | |
| M_User m_user = employeeMasterInter.getUserDetails(userID); | |
| ashaWorker.setEmployeeId(m_user.getUserID()); | |
| ashaWorker.setEmployeeId(m_user.getUserID()); | |
| ashaWorker.setDob(m_user.getDOB()); | |
| ashaWorker.setDateOfJoining(m_user.getDOJ()); | |
| ashaWorker.setName(m_user.getFirstName() + " " + m_user.getLastName()); | |
| ashaWorker.setMobileNumber(m_user.getContactNo()); | |
| ashaWorker.setAlternateMobileNumber(m_user.getEmergencyContactNo()); | |
| ashaWorker.setProviderServiceMapID(m_user.getServiceProviderID()); | |
| return ashaWorker; | |
| } | |
| private AshaWorker getDetails(Integer userID) { | |
| try { | |
| M_User m_user = Objects.requireNonNull( | |
| employeeMasterInter.getUserDetails(userID), | |
| "User details not found for ID: " + userID | |
| ); | |
| AshaWorker ashaWorker = new AshaWorker(); | |
| ashaWorker.setEmployeeId(m_user.getUserID()); | |
| ashaWorker.setDob(m_user.getDOB()); | |
| ashaWorker.setDateOfJoining(m_user.getDOJ()); | |
| ashaWorker.setName(String.format("%s %s", | |
| Objects.toString(m_user.getFirstName(), ""), | |
| Objects.toString(m_user.getLastName(), "")).trim()); | |
| ashaWorker.setMobileNumber(m_user.getContactNo()); | |
| ashaWorker.setAlternateMobileNumber(m_user.getEmergencyContactNo()); | |
| ashaWorker.setProviderServiceMapID(m_user.getServiceProviderID()); | |
| return ashaWorker; | |
| } catch (Exception e) { | |
| logger.error("Error creating ASHA worker profile from user details for ID {}: {}", userID, e.getMessage(), e); | |
| throw new RuntimeException("Failed to create ASHA worker profile from user details", e); | |
| } | |
| } |
| @Override | ||
| public AshaWorker getProfileData(Integer employeeId) { | ||
| if (ashaProfileRepo.findByEmployeeId(employeeId)!=null) { | ||
| return ashaProfileRepo.findByEmployeeId(employeeId); | ||
| } else { | ||
| return getDetails(employeeId); | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Optimize repository access and add error handling in getProfileData method.
The method makes redundant repository calls and lacks proper error handling:
@Override
public AshaWorker getProfileData(Integer employeeId) {
- if (ashaProfileRepo.findByEmployeeId(employeeId)!=null) {
- return ashaProfileRepo.findByEmployeeId(employeeId);
- } else {
- return getDetails(employeeId);
+ try {
+ Objects.requireNonNull(employeeId, "employeeId must not be null");
+ return ashaProfileRepo.findByEmployeeId(employeeId)
+ .orElseGet(() -> getDetails(employeeId));
+ } catch (Exception e) {
+ logger.error("Error retrieving ASHA worker profile for employeeId {}: {}", employeeId, e.getMessage(), e);
+ throw new RuntimeException("Failed to retrieve ASHA worker profile", e);
}
}📝 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.
| @Override | |
| public AshaWorker getProfileData(Integer employeeId) { | |
| if (ashaProfileRepo.findByEmployeeId(employeeId)!=null) { | |
| return ashaProfileRepo.findByEmployeeId(employeeId); | |
| } else { | |
| return getDetails(employeeId); | |
| } | |
| } | |
| @Override | |
| public AshaWorker getProfileData(Integer employeeId) { | |
| try { | |
| Objects.requireNonNull(employeeId, "employeeId must not be null"); | |
| return ashaProfileRepo.findByEmployeeId(employeeId) | |
| .orElseGet(() -> getDetails(employeeId)); | |
| } catch (Exception e) { | |
| logger.error("Error retrieving ASHA worker profile for employeeId {}: {}", employeeId, e.getMessage(), e); | |
| throw new RuntimeException("Failed to retrieve ASHA worker profile", e); | |
| } | |
| } |
| private AshaWorker updateProfile(AshaWorker editEmployee) { | ||
| System.out.println(editEmployee.toString()); | ||
|
|
||
| AshaWorker editdata = new AshaWorker(); | ||
| editdata.setId(editEmployee.getId()); | ||
| editdata.setAbhaNumber(editEmployee.getAbhaNumber()); | ||
| editdata.setEmployeeId(editEmployee.getEmployeeId()); | ||
| editdata.setDob(editEmployee.getDob()); | ||
| editdata.setAlternateMobileNumber(editEmployee.getAlternateMobileNumber()); | ||
| editdata.setAnm1Mobile(editEmployee.getAnm1Mobile()); | ||
| editdata.setAnm2Name(editEmployee.getAnm2Name()); | ||
| editdata.setIfsc(editEmployee.getIfsc()); | ||
| editdata.setAwwName(editEmployee.getAwwName()); | ||
| editdata.setName(editEmployee.getName()); | ||
| editdata.setVillage(editEmployee.getVillage()); | ||
| editdata.setBankAccount(editEmployee.getBankAccount()); | ||
| editdata.setChoName(editEmployee.getChoName()); | ||
| editdata.setChoMobile(editEmployee.getChoMobile()); | ||
| editdata.setAbhaNumber(editEmployee.getAbhaNumber()); | ||
| editdata.setAshaFamilyMember(editEmployee.getAshaFamilyMember()); | ||
| editdata.setDateOfJoining(editEmployee.getDateOfJoining()); | ||
| editdata.setMobileNumber(editEmployee.getMobileNumber()); | ||
| editdata.setAshaHouseholdRegistration(editEmployee.getAshaHouseholdRegistration()); | ||
| editdata.setFatherOrSpouseName(editEmployee.getFatherOrSpouseName()); | ||
| editdata.setPopulationCovered(editEmployee.getPopulationCovered()); | ||
| editdata.setAnm1Name(editEmployee.getAnm1Name()); | ||
| editdata.setAnm2Mobile(editEmployee.getAnm2Mobile()); // Corrected line | ||
| editdata.setAwwMobile(editEmployee.getAwwMobile()); | ||
| editdata.setProviderServiceMapID(editEmployee.getProviderServiceMapID()); | ||
|
|
||
| return editdata; | ||
|
|
||
|
|
||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve updateProfile method with proper logging and consider using a mapper.
The method has several issues:
- Uses
System.out.printlninstead of proper logging - Contains duplicate
setAbhaNumbercall (lines 71 and 84) - Manual field copying is error-prone
Consider using a mapper library like MapStruct to handle object mapping:
- private AshaWorker updateProfile(AshaWorker editEmployee) {
- System.out.println(editEmployee.toString());
-
- AshaWorker editdata = new AshaWorker();
- editdata.setId(editEmployee.getId());
- editdata.setAbhaNumber(editEmployee.getAbhaNumber());
- editdata.setEmployeeId(editEmployee.getEmployeeId());
- // ... more setters
- return editdata;
+ private AshaWorker updateProfile(AshaWorker editEmployee) {
+ try {
+ Objects.requireNonNull(editEmployee, "editEmployee must not be null");
+ logger.debug("Updating ASHA worker profile: {}", editEmployee);
+ return ashaWorkerMapper.update(editEmployee);
+ } catch (Exception e) {
+ logger.error("Error updating ASHA worker profile: {}", e.getMessage(), e);
+ throw new RuntimeException("Failed to update ASHA worker profile", e);
+ }
}If you prefer not to use a mapper library, at least fix the current implementation:
private AshaWorker updateProfile(AshaWorker editEmployee) {
- System.out.println(editEmployee.toString());
+ try {
+ Objects.requireNonNull(editEmployee, "editEmployee must not be null");
+ logger.debug("Updating ASHA worker profile: {}", editEmployee);
- AshaWorker editdata = new AshaWorker();
- editdata.setId(editEmployee.getId());
- editdata.setAbhaNumber(editEmployee.getAbhaNumber());
- editdata.setEmployeeId(editEmployee.getEmployeeId());
- editdata.setDob(editEmployee.getDob());
- editdata.setAlternateMobileNumber(editEmployee.getAlternateMobileNumber());
- editdata.setAnm1Mobile(editEmployee.getAnm1Mobile());
- editdata.setAnm2Name(editEmployee.getAnm2Name());
- editdata.setIfsc(editEmployee.getIfsc());
- editdata.setAwwName(editEmployee.getAwwName());
- editdata.setName(editEmployee.getName());
- editdata.setVillage(editEmployee.getVillage());
- editdata.setBankAccount(editEmployee.getBankAccount());
- editdata.setChoName(editEmployee.getChoName());
- editdata.setChoMobile(editEmployee.getChoMobile());
- editdata.setAbhaNumber(editEmployee.getAbhaNumber()); // Duplicate line
- editdata.setAshaFamilyMember(editEmployee.getAshaFamilyMember());
- editdata.setDateOfJoining(editEmployee.getDateOfJoining());
- editdata.setMobileNumber(editEmployee.getMobileNumber());
- editdata.setAshaHouseholdRegistration(editEmployee.getAshaHouseholdRegistration());
- editdata.setFatherOrSpouseName(editEmployee.getFatherOrSpouseName());
- editdata.setPopulationCovered(editEmployee.getPopulationCovered());
- editdata.setAnm1Name(editEmployee.getAnm1Name());
- editdata.setAnm2Mobile(editEmployee.getAnm2Mobile());
- editdata.setAwwMobile(editEmployee.getAwwMobile());
- editdata.setProviderServiceMapID(editEmployee.getProviderServiceMapID());
+ AshaWorker editdata = new AshaWorker();
+ editdata.setId(editEmployee.getId());
+ editdata.setEmployeeId(editEmployee.getEmployeeId());
+ editdata.setDob(editEmployee.getDob());
+ editdata.setAlternateMobileNumber(editEmployee.getAlternateMobileNumber());
+ editdata.setAnm1Mobile(editEmployee.getAnm1Mobile());
+ editdata.setAnm2Name(editEmployee.getAnm2Name());
+ editdata.setIfsc(editEmployee.getIfsc());
+ editdata.setAwwName(editEmployee.getAwwName());
+ editdata.setName(editEmployee.getName());
+ editdata.setVillage(editEmployee.getVillage());
+ editdata.setBankAccount(editEmployee.getBankAccount());
+ editdata.setChoName(editEmployee.getChoName());
+ editdata.setChoMobile(editEmployee.getChoMobile());
+ editdata.setAbhaNumber(editEmployee.getAbhaNumber());
+ editdata.setAshaFamilyMember(editEmployee.getAshaFamilyMember());
+ editdata.setDateOfJoining(editEmployee.getDateOfJoining());
+ editdata.setMobileNumber(editEmployee.getMobileNumber());
+ editdata.setAshaHouseholdRegistration(editEmployee.getAshaHouseholdRegistration());
+ editdata.setFatherOrSpouseName(editEmployee.getFatherOrSpouseName());
+ editdata.setPopulationCovered(editEmployee.getPopulationCovered());
+ editdata.setAnm1Name(editEmployee.getAnm1Name());
+ editdata.setAnm2Mobile(editEmployee.getAnm2Mobile());
+ editdata.setAwwMobile(editEmployee.getAwwMobile());
+ editdata.setProviderServiceMapID(editEmployee.getProviderServiceMapID());
- return editdata;
+ return editdata;
+ } catch (Exception e) {
+ logger.error("Error creating updated ASHA worker profile: {}", e.getMessage(), e);
+ throw new RuntimeException("Failed to create updated ASHA worker profile", e);
+ }
}📝 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.
| private AshaWorker updateProfile(AshaWorker editEmployee) { | |
| System.out.println(editEmployee.toString()); | |
| AshaWorker editdata = new AshaWorker(); | |
| editdata.setId(editEmployee.getId()); | |
| editdata.setAbhaNumber(editEmployee.getAbhaNumber()); | |
| editdata.setEmployeeId(editEmployee.getEmployeeId()); | |
| editdata.setDob(editEmployee.getDob()); | |
| editdata.setAlternateMobileNumber(editEmployee.getAlternateMobileNumber()); | |
| editdata.setAnm1Mobile(editEmployee.getAnm1Mobile()); | |
| editdata.setAnm2Name(editEmployee.getAnm2Name()); | |
| editdata.setIfsc(editEmployee.getIfsc()); | |
| editdata.setAwwName(editEmployee.getAwwName()); | |
| editdata.setName(editEmployee.getName()); | |
| editdata.setVillage(editEmployee.getVillage()); | |
| editdata.setBankAccount(editEmployee.getBankAccount()); | |
| editdata.setChoName(editEmployee.getChoName()); | |
| editdata.setChoMobile(editEmployee.getChoMobile()); | |
| editdata.setAbhaNumber(editEmployee.getAbhaNumber()); | |
| editdata.setAshaFamilyMember(editEmployee.getAshaFamilyMember()); | |
| editdata.setDateOfJoining(editEmployee.getDateOfJoining()); | |
| editdata.setMobileNumber(editEmployee.getMobileNumber()); | |
| editdata.setAshaHouseholdRegistration(editEmployee.getAshaHouseholdRegistration()); | |
| editdata.setFatherOrSpouseName(editEmployee.getFatherOrSpouseName()); | |
| editdata.setPopulationCovered(editEmployee.getPopulationCovered()); | |
| editdata.setAnm1Name(editEmployee.getAnm1Name()); | |
| editdata.setAnm2Mobile(editEmployee.getAnm2Mobile()); // Corrected line | |
| editdata.setAwwMobile(editEmployee.getAwwMobile()); | |
| editdata.setProviderServiceMapID(editEmployee.getProviderServiceMapID()); | |
| return editdata; | |
| } | |
| private AshaWorker updateProfile(AshaWorker editEmployee) { | |
| try { | |
| Objects.requireNonNull(editEmployee, "editEmployee must not be null"); | |
| logger.debug("Updating ASHA worker profile: {}", editEmployee); | |
| AshaWorker editdata = new AshaWorker(); | |
| editdata.setId(editEmployee.getId()); | |
| editdata.setEmployeeId(editEmployee.getEmployeeId()); | |
| editdata.setDob(editEmployee.getDob()); | |
| editdata.setAlternateMobileNumber(editEmployee.getAlternateMobileNumber()); | |
| editdata.setAnm1Mobile(editEmployee.getAnm1Mobile()); | |
| editdata.setAnm2Name(editEmployee.getAnm2Name()); | |
| editdata.setIfsc(editEmployee.getIfsc()); | |
| editdata.setAwwName(editEmployee.getAwwName()); | |
| editdata.setName(editEmployee.getName()); | |
| editdata.setVillage(editEmployee.getVillage()); | |
| editdata.setBankAccount(editEmployee.getBankAccount()); | |
| editdata.setChoName(editEmployee.getChoName()); | |
| editdata.setChoMobile(editEmployee.getChoMobile()); | |
| editdata.setAbhaNumber(editEmployee.getAbhaNumber()); | |
| editdata.setAshaFamilyMember(editEmployee.getAshaFamilyMember()); | |
| editdata.setDateOfJoining(editEmployee.getDateOfJoining()); | |
| editdata.setMobileNumber(editEmployee.getMobileNumber()); | |
| editdata.setAshaHouseholdRegistration(editEmployee.getAshaHouseholdRegistration()); | |
| editdata.setFatherOrSpouseName(editEmployee.getFatherOrSpouseName()); | |
| editdata.setPopulationCovered(editEmployee.getPopulationCovered()); | |
| editdata.setAnm1Name(editEmployee.getAnm1Name()); | |
| editdata.setAnm2Mobile(editEmployee.getAnm2Mobile()); | |
| editdata.setAwwMobile(editEmployee.getAwwMobile()); | |
| editdata.setProviderServiceMapID(editEmployee.getProviderServiceMapID()); | |
| return editdata; | |
| } catch (Exception e) { | |
| logger.error("Error creating updated ASHA worker profile: {}", e.getMessage(), e); | |
| throw new RuntimeException("Failed to create updated ASHA worker profile", e); | |
| } | |
| } |
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
src/main/java/com/iemr/flw/controller/AshaProfileController.java (3)
23-27:⚠️ Potential issueFix critical issue with shared mutable state.
The controller uses a shared mutable
responsemap that can cause race conditions when handling concurrent requests. Each request should have its own response map.public class AshaProfileController { private Logger logger = LoggerFactory.getLogger(this.getClass().getSimpleName()); @Autowired AshaProfileService ashaProfileService; - private Map<String,Object> response = new HashMap<>();
35-60:⚠️ Potential issueFix critical issues in error handling and response management.
The
editEmployeemethod has several critical issues that need to be addressed:
- Using a shared mutable state (response map)
- Incorrect error handling (creating but not returning a ResponseEntity in the catch block)
- Using System.out.println instead of proper logging
@RequestMapping(value = "editProfile", method = { RequestMethod.POST }, produces = { "application/json" },consumes = "application/json" ) public ResponseEntity<Map<String,Object>> editEmployee(@RequestBody AshaWorker editEmployee) { - + Map<String,Object> response = new HashMap<>(); try { - System.out.println(editEmployee.toString()); + logger.debug("Editing employee profile: {}", editEmployee); AshaWorker ashaWorker = ashaProfileService.saveEditData(editEmployee); response.put("data",ashaWorker); response.put("statusCode",200); response.put("status","Success"); response.put("errorMessage","Success"); - - - - + return ResponseEntity.ok().body(response); } catch (Exception e) { logger.error("Unexpected error:", e); - ResponseEntity.status(500).body(e.getMessage()); + response.put("statusCode", 500); + response.put("status", "Error"); + response.put("errorMessage", e.getMessage()); + return ResponseEntity.status(500).body(response); } - - return ResponseEntity.ok().body(response); }
61-87:⚠️ Potential issueFix similar critical issues in the getProfile method.
The
getProfilemethod has the same issues aseditEmployee: shared mutable state, incorrect error handling, and always returning a success response regardless of errors.@Operation(summary = "Profile Detail") @RequestMapping(value = "getProfile",method = RequestMethod.GET) public ResponseEntity<Map<String,Object>> getProfile(@RequestParam ("employeeId")Integer employeeId){ + Map<String,Object> response = new HashMap<>(); try { + if (employeeId == null) { + throw new IllegalArgumentException("Employee ID cannot be null"); + } AshaWorker ashaWorker = ashaProfileService.getProfileData(employeeId); if(ashaWorker!=null){ response.put("data",ashaWorker); response.put("statusCode",200); response.put("status","Success"); response.put("errorMessage","Success"); }else { response.put("data",ashaWorker); response.put("statusCode",200); response.put("status","Success"); response.put("errorMessage","Asha profile not found"); } + return ResponseEntity.ok().body(response); }catch (Exception e) { logger.error("Unexpected error:", e); - ResponseEntity.status(500).body(e.getMessage()); + response.put("statusCode", 500); + response.put("status", "Error"); + response.put("errorMessage", e.getMessage()); + return ResponseEntity.status(500).body(response); } - - return ResponseEntity.ok().body(response); - - }
🧹 Nitpick comments (1)
src/main/java/com/iemr/flw/controller/AshaProfileController.java (1)
24-24: Use class name directly in Logger initialization.Instead of using
this.getClass().getSimpleName(), use the class name directly for better logging performance.- private Logger logger = LoggerFactory.getLogger(this.getClass().getSimpleName()); + private static final Logger logger = LoggerFactory.getLogger(AshaProfileController.class);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/main/java/com/iemr/flw/controller/AshaProfileController.java(1 hunks)src/main/java/com/iemr/flw/repo/iemr/AshaProfileRepo.java(1 hunks)src/main/java/com/iemr/flw/service/AshaProfileService.java(1 hunks)src/main/java/com/iemr/flw/service/impl/AshaProfileImpl.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- src/main/java/com/iemr/flw/repo/iemr/AshaProfileRepo.java
- src/main/java/com/iemr/flw/service/AshaProfileService.java
- src/main/java/com/iemr/flw/service/impl/AshaProfileImpl.java
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/main/java/com/iemr/flw/domain/iemr/ANCVisit.java (1)
115-118: Field addition looks good but consider adding documentation and constraints.The addition of the
filePathfield with proper column mapping is appropriate for storing file paths associated with ANC visits. However, consider:
- Adding a Javadoc comment to explain what type of files this path will reference
- Adding a maximum length constraint to the column annotation (e.g.,
@Column(name = "file_path", length = 255)) to prevent potential database issues with extremely long paths- Ensuring there's a corresponding database migration script to add this column to the existing table
+ /** + * Path to the file uploaded during the ANC visit (e.g., ultrasound report, test results) + */ @Column(name = "file_path") private String filePath;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/main/java/com/iemr/flw/controller/AshaProfileController.java(1 hunks)src/main/java/com/iemr/flw/domain/iemr/ANCVisit.java(1 hunks)src/main/java/com/iemr/flw/dto/iemr/ANCVisitDTO.java(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/main/java/com/iemr/flw/dto/iemr/ANCVisitDTO.java
- src/main/java/com/iemr/flw/controller/AshaProfileController.java
📋 Description
JIRA ID:
Work on asha update profile and get profile , lmp date
✅ 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
Chores