#CCSD-389 Birth& death module integrated with user service for applicant #331
Conversation
publish check reciepts
Dev build 12
updarte pkg json
revert changes
Enc client version upgraded
CCSD-52 FIX
Solved Firenoc issue in mono-ui and some pgr issues
Ccsd 52 fix 2
Isne 726 662
Version update
ISNE-662 with pkg update
fixed the bug
fixed the sla days in table
JithendarKumar-eGov
requested review from
kavi-egov,
pradeepkumarcm-egov and
vinothrallapalli-eGov
as code owners
|
Important Review skippedMore than 25% of the files skipped due to max files limit. The review is being skipped to prevent a low-quality review. 125 files out of 234 files are above the max files limit of 100. Please upgrade to Pro plan to get higher limits. You can disable this status message by setting the WalkthroughThis update introduces a comprehensive user management module to the birth and death registration services. It adds new user model classes, service logic for user creation, update, and enrichment, and interfaces for parent information abstraction. SQL queries and row mappers are modified to include and process the father's mobile number. The application is refactored to use the new user models, with decryption and user enrichment logic integrated into search and download operations. Configuration files and properties are updated to support user service integration. Changes
Sequence Diagram(s)sequenceDiagram
participant Controller
participant Service
participant UserService
participant UserAPI
participant Repo
participant EncryptionUtil
Controller->>Service: search(criteria, requestInfo)
Service->>Repo: getDetailsAll(criteria)
Repo-->>Service: List<EgBirthDtl/EgDeathDtl>
Service->>EncryptionUtil: decrypt(detailsList)
EncryptionUtil-->>Service: decrypted detailsList
Service->>UserService: getOwners(detailsList, requestInfo)
UserService->>UserAPI: POST /user/_search (mobile numbers)
UserAPI-->>UserService: UserDetailResponse
UserService-->>Service: UserDetailResponse
Service->>Service: Map users to details by mobile number
Service-->>Controller: List<EgBirthDtl/EgDeathDtl> (enriched)
sequenceDiagram
participant Service
participant UserService
participant UserAPI
Service->>UserService: manageOwner(tenantId, fatherInfo, requestInfo, isUpdate)
UserService->>UserAPI: POST /user/_search (mobile number, name)
UserAPI-->>UserService: UserDetailResponse
alt user not found
UserService->>UserAPI: POST /user/_create
else user found and isUpdate
UserService->>UserAPI: POST /user/_update
end
UserAPI-->>UserService: UserDetailResponse
UserService-->>Service: User
Poem
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 37
🔭 Outside diff range comments (3)
birth-death-services/src/main/java/org/bel/birthdeath/birth/service/EnrichmentService.java (1)
110-123: Add null safety checks to prevent potential exceptionsThe code assumes
birtDtlsis non-null and non-empty when the user type is not "CITIZEN". This could lead toNullPointerExceptionorIndexOutOfBoundsException.Apply this diff to add proper null checks:
public void setDemandParams(BirthCertRequest birthCertRequest ,List<EgBirthDtl> birtDtls) { BirthCertificate birthCert = birthCertRequest.getBirthCertificate(); birthCert.setBusinessService(BIRTH_CERT); ArrayList<Amount> amounts = new ArrayList<>(); Amount amount=new Amount(); amount.setTaxHeadCode(BIRTH_CERT_FEE); amount.setAmount(new BigDecimal(50)); amounts.add(amount); birthCert.setAmount(amounts); if (birthCertRequest.getRequestInfo().getUserInfo().getType().equalsIgnoreCase("CITIZEN")) birthCert.setCitizen(convertToCustomUser(birthCertRequest.getRequestInfo().getUserInfo())); - else - birthCert.setCitizen(birtDtls.get(0).getUser()); + else if (birtDtls != null && !birtDtls.isEmpty() && birtDtls.get(0).getUser() != null) + birthCert.setCitizen(birtDtls.get(0).getUser()); birthCert.setTaxPeriodFrom(System.currentTimeMillis()); birthCert.setTaxPeriodTo(System.currentTimeMillis()+86400000); }birth-death-services/src/main/java/org/bel/birthdeath/death/service/DeathService.java (1)
194-197: Improve exception handling specificityThe catch block is too generic and loses valuable debugging information by catching all exceptions.
Consider logging the actual exception and being more specific about error handling:
-catch(Exception e) { - e.printStackTrace(); - throw new CustomException("DOWNLOAD_ERROR","Error in Downloading Certificate"); -} +catch(Exception e) { + log.error("Error in downloading death certificate for ID: {} in tenant: {}", + criteria.getId(), criteria.getTenantId(), e); + throw new CustomException("DOWNLOAD_ERROR", + "Error in downloading certificate: " + e.getMessage(), e); +}birth-death-services/src/main/java/org/bel/birthdeath/death/service/EnrichmentServiceDeath.java (1)
119-141: Consider extracting citizen resolution logic for better readabilityThe nested if-else logic for determining the citizen is complex and could be extracted into a separate method.
public void setDemandParams(DeathCertRequest deathCertRequest,List<EgDeathDtl> deathDtls) { DeathCertificate deathCert = deathCertRequest.getDeathCertificate(); deathCert.setBusinessService(DEATH_CERT); ArrayList<Amount> amounts = new ArrayList<>(); Amount amount=new Amount(); amount.setTaxHeadCode(DEATH_CERT_FEE); amount.setAmount(new BigDecimal(50)); amounts.add(amount); deathCert.setAmount(amounts); - if(deathCertRequest.getRequestInfo().getUserInfo().getType().equalsIgnoreCase("CITIZEN")) - deathCert.setCitizen(convertToCustomUser(deathCertRequest.getRequestInfo().getUserInfo())); - else if (deathDtls != null && !deathDtls.isEmpty() && deathDtls.get(0).getUser() != null) { - deathCert.setCitizen(deathDtls.get(0).getUser()); - } else{ - // fetch user using the id sent in deathcertRequest and then pass that user - UserSearchRequest userSearchRequest = getBaseUserSearchRequest(deathCertRequest.getRequestInfo().getUserInfo().getTenantId(), deathCertRequest.getRequestInfo()); - UserDetailResponse userDetailResponse = getUser(userSearchRequest); - deathCert.setCitizen(userDetailResponse.getUser().get(0)); - } + deathCert.setCitizen(resolveCitizen(deathCertRequest, deathDtls)); deathCert.setTaxPeriodFrom(System.currentTimeMillis()); deathCert.setTaxPeriodTo(System.currentTimeMillis()+86400000); } +private User resolveCitizen(DeathCertRequest deathCertRequest, List<EgDeathDtl> deathDtls) { + if(deathCertRequest.getRequestInfo().getUserInfo().getType().equalsIgnoreCase("CITIZEN")) { + return convertToCustomUser(deathCertRequest.getRequestInfo().getUserInfo()); + } + + if (deathDtls != null && !deathDtls.isEmpty() && deathDtls.get(0).getUser() != null) { + return deathDtls.get(0).getUser(); + } + + UserSearchRequest userSearchRequest = getBaseUserSearchRequest( + deathCertRequest.getRequestInfo().getUserInfo().getTenantId(), + deathCertRequest.getRequestInfo() + ); + UserDetailResponse userDetailResponse = getUser(userSearchRequest); + + if (userDetailResponse != null && userDetailResponse.getUser() != null && !userDetailResponse.getUser().isEmpty()) { + return userDetailResponse.getUser().get(0); + } + + return null; +}
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (38)
birth-death-services/pom.xml(2 hunks)birth-death-services/src/main/java/org/bel/birthdeath/BirthDeathApplication.java(2 hunks)birth-death-services/src/main/java/org/bel/birthdeath/birth/certmodel/BirthCertificate.java(1 hunks)birth-death-services/src/main/java/org/bel/birthdeath/birth/model/EgBirthDtl.java(3 hunks)birth-death-services/src/main/java/org/bel/birthdeath/birth/model/EgBirthFatherInfo.java(1 hunks)birth-death-services/src/main/java/org/bel/birthdeath/birth/repository/BirthRepository.java(1 hunks)birth-death-services/src/main/java/org/bel/birthdeath/birth/repository/builder/BirthDtlAllQueryBuilder.java(3 hunks)birth-death-services/src/main/java/org/bel/birthdeath/birth/repository/rowmapper/BirthDtlsAllRowMapper.java(1 hunks)birth-death-services/src/main/java/org/bel/birthdeath/birth/service/BirthService.java(7 hunks)birth-death-services/src/main/java/org/bel/birthdeath/birth/service/DemandService.java(1 hunks)birth-death-services/src/main/java/org/bel/birthdeath/birth/service/EnrichmentService.java(4 hunks)birth-death-services/src/main/java/org/bel/birthdeath/common/calculation/demand/models/Demand.java(1 hunks)birth-death-services/src/main/java/org/bel/birthdeath/common/config/CommonConfiguration.java(1 hunks)birth-death-services/src/main/java/org/bel/birthdeath/common/contract/DeathResponse.java(1 hunks)birth-death-services/src/main/java/org/bel/birthdeath/common/contract/ParentInfo.java(1 hunks)birth-death-services/src/main/java/org/bel/birthdeath/common/contract/ParentInfoProvider.java(1 hunks)birth-death-services/src/main/java/org/bel/birthdeath/common/model/user/User.java(1 hunks)birth-death-services/src/main/java/org/bel/birthdeath/common/model/user/UserDetailResponse.java(1 hunks)birth-death-services/src/main/java/org/bel/birthdeath/common/model/user/UserRequest.java(1 hunks)birth-death-services/src/main/java/org/bel/birthdeath/common/model/user/UserSearchRequest.java(1 hunks)birth-death-services/src/main/java/org/bel/birthdeath/common/services/UserService.java(1 hunks)birth-death-services/src/main/java/org/bel/birthdeath/common/util/CommonErrorConstants.java(1 hunks)birth-death-services/src/main/java/org/bel/birthdeath/common/util/Constants.java(1 hunks)birth-death-services/src/main/java/org/bel/birthdeath/common/util/UserUtil.java(1 hunks)birth-death-services/src/main/java/org/bel/birthdeath/config/BirthDeathConfiguration.java(1 hunks)birth-death-services/src/main/java/org/bel/birthdeath/death/certmodel/DeathCertRequest.java(1 hunks)birth-death-services/src/main/java/org/bel/birthdeath/death/certmodel/DeathCertificate.java(1 hunks)birth-death-services/src/main/java/org/bel/birthdeath/death/model/EgDeathDtl.java(3 hunks)birth-death-services/src/main/java/org/bel/birthdeath/death/model/EgDeathFatherInfo.java(3 hunks)birth-death-services/src/main/java/org/bel/birthdeath/death/model/UserSearchRequest.java(1 hunks)birth-death-services/src/main/java/org/bel/birthdeath/death/repository/DeathRepository.java(1 hunks)birth-death-services/src/main/java/org/bel/birthdeath/death/repository/builder/DeathDtlAllQueryBuilder.java(2 hunks)birth-death-services/src/main/java/org/bel/birthdeath/death/repository/rowmapper/DeathDtlsAllRowMapper.java(1 hunks)birth-death-services/src/main/java/org/bel/birthdeath/death/repository/rowmapper/DeathDtlsRowMapper.java(1 hunks)birth-death-services/src/main/java/org/bel/birthdeath/death/service/DeathService.java(6 hunks)birth-death-services/src/main/java/org/bel/birthdeath/death/service/DemandServiceDeath.java(1 hunks)birth-death-services/src/main/java/org/bel/birthdeath/death/service/EnrichmentServiceDeath.java(6 hunks)birth-death-services/src/main/resources/application.properties(6 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (7)
birth-death-services/src/main/java/org/bel/birthdeath/birth/model/EgBirthFatherInfo.java (2)
birth-death-services/src/main/java/org/bel/birthdeath/birth/model/EgBirthDtl.java (1)
Getter(16-104)birth-death-services/src/main/java/org/bel/birthdeath/death/model/EgDeathFatherInfo.java (1)
Getter(11-54)
birth-death-services/src/main/java/org/bel/birthdeath/death/model/EgDeathFatherInfo.java (2)
birth-death-services/src/main/java/org/bel/birthdeath/death/model/EgDeathDtl.java (1)
Getter(16-118)birth-death-services/src/main/java/org/bel/birthdeath/birth/model/EgBirthFatherInfo.java (1)
Getter(11-46)
birth-death-services/src/main/java/org/bel/birthdeath/common/model/user/UserSearchRequest.java (4)
birth-death-services/src/main/java/org/bel/birthdeath/common/model/user/UserDetailResponse.java (1)
AllArgsConstructor(10-22)birth-death-services/src/main/java/org/bel/birthdeath/common/model/user/UserRequest.java (1)
AllArgsConstructor(12-25)birth-death-services/src/main/java/org/bel/birthdeath/common/config/CommonConfiguration.java (1)
Getter(12-39)frontend/micro-ui/web/micro-ui-internals/packages/libraries/src/services/atoms/Utils/Request.js (1)
requestInfo(40-42)
birth-death-services/src/main/java/org/bel/birthdeath/common/model/user/UserRequest.java (6)
birth-death-services/src/main/java/org/bel/birthdeath/common/model/user/UserDetailResponse.java (1)
AllArgsConstructor(10-22)birth-death-services/src/main/java/org/bel/birthdeath/common/model/user/UserSearchRequest.java (1)
AllArgsConstructor(18-79)birth-death-services/src/main/java/org/bel/birthdeath/common/contract/DeathResponse.java (1)
Getter(19-36)birth-death-services/src/main/java/org/bel/birthdeath/death/certmodel/DeathCertRequest.java (1)
Getter(15-32)birth-death-services/src/main/java/org/bel/birthdeath/common/config/CommonConfiguration.java (1)
Getter(12-39)frontend/micro-ui/web/micro-ui-internals/packages/libraries/src/services/atoms/Utils/Request.js (1)
requestInfo(40-42)
birth-death-services/src/main/java/org/bel/birthdeath/common/model/user/User.java (7)
utilities/report/src/main/java/org/egov/swagger/model/Role.java (1)
Role(15-102)birth-death-services/src/main/java/org/bel/birthdeath/common/model/user/UserDetailResponse.java (1)
AllArgsConstructor(10-22)birth-death-services/src/main/java/org/bel/birthdeath/common/model/user/UserRequest.java (1)
AllArgsConstructor(12-25)birth-death-services/src/main/java/org/bel/birthdeath/birth/model/EgBirthDtl.java (1)
Getter(16-104)birth-death-services/src/main/java/org/bel/birthdeath/death/model/EgDeathDtl.java (1)
Getter(16-118)birth-death-services/src/main/java/org/bel/birthdeath/birth/certmodel/BirthCertificate.java (1)
Getter(20-145)birth-death-services/src/main/java/org/bel/birthdeath/death/certmodel/DeathCertificate.java (1)
Getter(20-147)
birth-death-services/src/main/java/org/bel/birthdeath/death/model/EgDeathDtl.java (2)
birth-death-services/src/main/java/org/bel/birthdeath/birth/model/EgBirthDtl.java (1)
Getter(16-104)birth-death-services/src/main/java/org/bel/birthdeath/death/model/EgDeathFatherInfo.java (1)
Getter(11-54)
birth-death-services/src/main/java/org/bel/birthdeath/birth/model/EgBirthDtl.java (1)
birth-death-services/src/main/java/org/bel/birthdeath/birth/model/EgBirthFatherInfo.java (1)
Getter(11-46)
🔇 Additional comments (38)
birth-death-services/src/main/java/org/bel/birthdeath/config/BirthDeathConfiguration.java (1)
126-126: EOF newline addition is fineAdding a trailing newline is a harmless change and aligns with typical POSIX/editorconfig guidelines.
birth-death-services/src/main/java/org/bel/birthdeath/birth/service/DemandService.java (1)
17-18: Import correctly migrated to newUserdomainSwitching to
org.bel.birthdeath.common.model.user.Userkeeps the service aligned with the new user model. Looks good.birth-death-services/src/main/java/org/bel/birthdeath/common/calculation/demand/models/Demand.java (1)
19-20: Import aligned with unified user modelNo further changes required; serialization contracts remain unaffected.
birth-death-services/src/main/java/org/bel/birthdeath/birth/certmodel/BirthCertificate.java (1)
9-9: Consistent User model migration - looks good.The import change is consistent with the DeathCertificate class and properly aligns with the overall migration to local User models.
birth-death-services/src/main/java/org/bel/birthdeath/common/contract/DeathResponse.java (1)
24-36: Code formatting improvements.The formatting cleanup improves code consistency and readability without affecting functionality.
birth-death-services/src/main/java/org/bel/birthdeath/death/repository/rowmapper/DeathDtlsAllRowMapper.java (1)
42-43: Confirm database schema support forbfat.mobilenoThe code changes correctly extract and map
bfat.mobileno:
- SQL queries in DeathDtlAllQueryBuilder are updated to select
bfat.mobileno AS bfatmobileno.- RowMappers (DeathDtlsAllRowMapper, DeathDtlsRowMapper, DeathMasterDtlRowMapper) now call
rs.getString("bfatmobileno").- EgDeathFatherInfo.java declares a private
String mobileno;with getter.Please ensure the underlying DB schema has the
bfat.mobilenocolumn and that any migration scripts or changelogs have been applied to add this column.birth-death-services/src/main/java/org/bel/birthdeath/death/certmodel/DeathCertRequest.java (1)
12-12: Import organization improvement.The import reordering improves code organization by grouping imports logically.
birth-death-services/src/main/java/org/bel/birthdeath/death/repository/rowmapper/DeathDtlsRowMapper.java (1)
40-41: LGTM! Clean addition of father's mobile number extraction.The mobile number field is correctly extracted from the result set and follows the established pattern used for other father information fields.
birth-death-services/src/main/java/org/bel/birthdeath/death/repository/builder/DeathDtlAllQueryBuilder.java (1)
49-49: LGTM! Clean addition of father's mobile number to SQL queries.The father's mobile number field is correctly added to both query variants with:
- Proper table alias usage (
bfat.mobileno)- Consistent column alias (
bfatmobileno)- Appropriate placement in the SELECT clauses
This matches the expectations of the corresponding row mappers in the death module.
Also applies to: 67-67
birth-death-services/src/main/java/org/bel/birthdeath/death/service/DemandServiceDeath.java (1)
9-9: LGTM! Import change aligns with user service integration.The change from external contract
Userto local modelUseris consistent with the broader refactoring to integrate user service functionality.birth-death-services/src/main/java/org/bel/birthdeath/birth/repository/builder/BirthDtlAllQueryBuilder.java (1)
52-52: Fix duplicate mobile number selection in SQL query.Same duplication issue as in QUERY_MASTER_FULL_ALL - father's mobile number is selected twice with different aliases.
-bfat.firstname bfatfn ,bfat.mobileno bfatfmobileno, bmot.firstname bmotfn , bdtl.firstname bdtlfn , +bfat.firstname bfatfn , bmot.firstname bmotfn , bdtl.firstname bdtlfn ,Likely an incorrect or invalid review comment.
birth-death-services/src/main/java/org/bel/birthdeath/BirthDeathApplication.java (1)
16-16: LGTM! Primary ObjectMapper bean configuration is appropriate.Adding
@Primaryto theobjectMapperBnd()bean ensures consistent JSON processing behavior when multiple ObjectMapper beans are present, which aligns with the enhanced user service integration.Also applies to: 37-37
birth-death-services/src/main/java/org/bel/birthdeath/common/contract/ParentInfoProvider.java (1)
1-7: LGTM! Clean interface design for parent information abstraction.The interface provides a clear contract for accessing tenant ID and father information, enabling consistent user management across birth and death domain models.
birth-death-services/src/main/java/org/bel/birthdeath/birth/model/EgBirthFatherInfo.java (3)
9-9: LGTM! Proper import for ParentInfo interface.Clean import statement for the interface implementation.
17-17: LGTM! Correct interface implementation.The class properly implements the
ParentInfointerface, consistent with the similar implementation inEgDeathFatherInfo.
36-46: LGTM! Clean interface method implementations.The explicit
@Overridemethods correctly return the existing fields, providing the required contract methods for theParentInfointerface. The implementation is consistent with the pattern used inEgDeathFatherInfo.birth-death-services/src/main/java/org/bel/birthdeath/death/model/EgDeathFatherInfo.java (4)
9-9: LGTM: Interface implementation follows established pattern.The import of
ParentInfointerface supports the standardization of parent information access across the codebase.
17-17: LGTM: Consistent interface implementation.The implementation of
ParentInfointerface follows the same pattern asEgBirthFatherInfo, ensuring consistency across birth and death modules.
27-28: LGTM: Tenant ID field addition supports user management integration.The addition of
tenantidfield aligns with the broader user management and multi-tenancy requirements.
45-53: LGTM: Interface method overrides are correctly implemented.The explicit overrides for
getFirstname()andgetMobileno()properly expose the existing fields through theParentInfointerface, enabling polymorphic access to parent information.birth-death-services/pom.xml (1)
51-56: LGTM: Lombok dependency properly configured.The version update to 1.18.30 and scope change to
providedis correct for Lombok dependencies, ensuring it's only used during compilation.birth-death-services/src/main/java/org/bel/birthdeath/birth/model/EgBirthDtl.java (4)
5-5: Good addition of JsonIgnoreProperties for backward compatibility.The
@JsonIgnoreProperties(ignoreUnknown = true)annotation ensures that any unknown JSON properties during deserialization won't cause errors, providing good backward compatibility as the user integration evolves.Also applies to: 22-22
23-23: LGTM: Clean interface implementation.The class correctly implements
ParentInfoProvider, which provides a clean abstraction for accessing parent information across birth and death records.
79-79: Good addition of User field for enrichment.The
User userfield enables user data enrichment as described in the AI summary, allowing birth records to be associated with user information.
95-103: Interface method implementations look correct.The
getTenantid()andgetFatherInfo()methods correctly implement theParentInfoProviderinterface by returning the existing fields. This provides a consistent way to access tenant and father information across different record types.birth-death-services/src/main/java/org/bel/birthdeath/death/model/EgDeathDtl.java (4)
5-5: LGTM: Consistent with birth model changes.The
@JsonIgnoreProperties(ignoreUnknown = true)annotation is applied consistently with the birth model for backward compatibility.Also applies to: 22-22
23-23: Good consistency: Same interface implementation as birth model.The
ParentInfoProviderinterface implementation maintains consistency between birth and death models, enabling unified handling of parent information.
77-77: User field addition is consistent.The
User userfield is added consistently with the birth model, enabling user data enrichment for death records as well.
109-117: Correct interface implementation for death model.The methods correctly return
this.tenantidandthis.deathFatherInfo(note the death prefix), which is appropriate for the death model context.birth-death-services/src/main/resources/application.properties (5)
3-3: Consider implications of fixed port configuration.Changing from
server.port=0(random port) toserver.port=8080(fixed port) may cause port conflicts in environments where multiple instances run. Ensure this aligns with your deployment strategy.
8-8: Verify database name change is intentional.The database name changed from
bel_cb_devtopostgres. This appears to be an environment-specific change - ensure this is correct for your target environment and that the database exists.Also applies to: 13-13
20-20: Flyway enabled - ensure migration scripts are ready.Flyway was enabled (
spring.flyway.enabled=true). Verify that all necessary migration scripts are in place atclasspath:/db/migration/mainand tested.
97-97: State tenant change from pb to pg.The state-level tenant ID changed from
pbtopg. Ensure this change is reflected in your MDMS data and other configuration files that reference the tenant ID.Also applies to: 108-109
123-131: Good addition of user service configuration.The new user service configuration properties are well-structured and support the user integration functionality described in the PR objectives.
birth-death-services/src/main/java/org/bel/birthdeath/common/model/user/UserDetailResponse.java (1)
1-22: LGTM: Well-structured DTO class.The
UserDetailResponseclass follows good practices:
- Proper Lombok annotations for boilerplate code generation
- Correct JSON property annotations for API compatibility
- Standard response structure with
ResponseInfoand data payload- Consistent with other response classes in the system
birth-death-services/src/main/java/org/bel/birthdeath/common/model/user/UserRequest.java (1)
1-25: LGTM: Well-structured request class.The
UserRequestclass follows good practices:
- Proper Lombok annotations for clean code
- Correct JSON property annotations
- Standard request structure with
RequestInfoand data payload- Consistent with the pattern shown in relevant code snippets like
DeathCertRequestbirth-death-services/src/main/java/org/bel/birthdeath/birth/service/BirthService.java (2)
59-78: LGTM!The new service dependencies are properly autowired following Spring conventions.
163-163: LGTM!The updated method signature properly passes the birth details for demand parameter enrichment.
| public interface ParentInfo { | ||
| String getFirstname(); | ||
| String getMobileno(); |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Non-idiomatic method names
getFirstname() and getMobileno() diverge from typical Java-bean casing (getFirstName, getMobileNo / getMobileNumber). Consistent naming prevents IDE inspection noise and future refactors.
🤖 Prompt for AI Agents
In
birth-death-services/src/main/java/org/bel/birthdeath/common/contract/ParentInfo.java
around lines 3 to 5, the method names getFirstname() and getMobileno() do not
follow standard Java bean naming conventions. Rename getFirstname() to
getFirstName() and getMobileno() to getMobileNo() or getMobileNumber() to ensure
consistent camelCase naming. Update all references to these methods accordingly
to maintain consistency and avoid IDE inspection warnings.
birth-death-services/src/main/java/org/bel/birthdeath/common/model/user/UserSearchRequest.java
Show resolved
Hide resolved
| import lombok.AllArgsConstructor; | ||
| import lombok.Builder; | ||
| import lombok.EqualsAndHashCode; | ||
| import lombok.Getter; | ||
| import lombok.NoArgsConstructor; | ||
| import lombok.Setter; | ||
| import lombok.ToString; | ||
|
|
There was a problem hiding this comment.
Missing import for @Builder.Default
Fields below use @Builder.Default but the import is absent, causing compilation failure.
import lombok.ToString;
+import lombok.Builder.Default;📝 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.
| import lombok.AllArgsConstructor; | |
| import lombok.Builder; | |
| import lombok.EqualsAndHashCode; | |
| import lombok.Getter; | |
| import lombok.NoArgsConstructor; | |
| import lombok.Setter; | |
| import lombok.ToString; | |
| import lombok.AllArgsConstructor; | |
| import lombok.Builder; | |
| import lombok.EqualsAndHashCode; | |
| import lombok.Getter; | |
| import lombok.NoArgsConstructor; | |
| import lombok.Setter; | |
| import lombok.ToString; | |
| import lombok.Builder.Default; |
🤖 Prompt for AI Agents
In
birth-death-services/src/main/java/org/bel/birthdeath/common/model/user/UserSearchRequest.java
around lines 10 to 17, the import statement for lombok.Builder.Default is
missing, which causes compilation errors when using @Builder.Default on fields.
Add the import statement for lombok.Builder.Default along with the existing
lombok imports to resolve the issue.
| @JsonProperty("pageNumber") | ||
| private int pageNumber = 0; | ||
|
|
||
| @JsonProperty("sort") | ||
| private List<String> sort = Collections.singletonList("name"); | ||
|
|
There was a problem hiding this comment.
Default values ignored by Lombok builder
Without @Builder.Default, pageNumber and especially sort will be null in objects built via the builder, risking NPEs.
- @JsonProperty("pageNumber")
- private int pageNumber = 0;
+ @JsonProperty("pageNumber")
+ @Builder.Default
+ private int pageNumber = 0;
- @JsonProperty("sort")
- private List<String> sort = Collections.singletonList("name");
+ @JsonProperty("sort")
+ @Builder.Default
+ private List<String> sort = Collections.singletonList("name");📝 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.
| @JsonProperty("pageNumber") | |
| private int pageNumber = 0; | |
| @JsonProperty("sort") | |
| private List<String> sort = Collections.singletonList("name"); | |
| @JsonProperty("pageNumber") | |
| @Builder.Default | |
| private int pageNumber = 0; | |
| @JsonProperty("sort") | |
| @Builder.Default | |
| private List<String> sort = Collections.singletonList("name"); |
🤖 Prompt for AI Agents
In
birth-death-services/src/main/java/org/bel/birthdeath/common/model/user/UserSearchRequest.java
around lines 67 to 72, the default values for pageNumber and sort are ignored
when using Lombok's builder, causing them to be null and potentially leading to
NullPointerExceptions. To fix this, add the @Builder.Default annotation to both
fields so that their default values are preserved when objects are created via
the builder.
| package org.bel.birthdeath.death.model; | ||
|
|
||
| import java.util.Collections; | ||
| import java.util.List; | ||
| import java.util.Set; | ||
|
|
||
| import org.egov.common.contract.request.RequestInfo; | ||
|
|
||
| import com.fasterxml.jackson.annotation.JsonProperty; | ||
|
|
||
| import lombok.AllArgsConstructor; | ||
| import lombok.Builder; | ||
| import lombok.Builder.Default; | ||
| import lombok.Getter; | ||
| import lombok.NoArgsConstructor; | ||
| import lombok.Setter; | ||
|
|
||
| @Getter | ||
| @Setter | ||
| @NoArgsConstructor | ||
| @AllArgsConstructor | ||
| @Builder | ||
| public class UserSearchRequest { | ||
|
|
||
| @JsonProperty("RequestInfo") | ||
| private RequestInfo requestInfo; | ||
|
|
||
| @JsonProperty("uuid") | ||
| private Set<String> uuid; | ||
|
|
||
| @JsonProperty("id") | ||
| private List<String> id; | ||
|
|
||
| @JsonProperty("userName") | ||
| private String userName; | ||
|
|
||
| @JsonProperty("name") | ||
| private String name; | ||
|
|
||
| @JsonProperty("mobileNumber") | ||
| private String mobileNumber; | ||
|
|
||
| @JsonProperty("aadhaarNumber") | ||
| private String aadhaarNumber; | ||
|
|
||
| @JsonProperty("pan") | ||
| private String pan; | ||
|
|
||
| @JsonProperty("emailId") | ||
| private String emailId; | ||
|
|
||
| @JsonProperty("fuzzyLogic") | ||
| private boolean fuzzyLogic; | ||
|
|
||
| @JsonProperty("active") | ||
| @Setter | ||
| private Boolean active; | ||
|
|
||
| @JsonProperty("tenantId") | ||
| private String tenantId; | ||
|
|
||
| @JsonProperty("pageSize") | ||
| private int pageSize; | ||
|
|
||
| @JsonProperty("pageNumber") | ||
| @Default | ||
| private int pageNumber = 0; | ||
|
|
||
| @JsonProperty("sort") | ||
| @Default | ||
| private List<String> sort = Collections.singletonList("name"); | ||
|
|
||
| @JsonProperty("userType") | ||
| private String userType; | ||
|
|
||
| @JsonProperty("roleCodes") | ||
| private List<String> roleCodes; | ||
|
|
||
|
|
||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Redundant DTO – unify with common model
This class replicates common.model.user.UserSearchRequest. Prefer one authoritative definition.
🤖 Prompt for AI Agents
In
birth-death-services/src/main/java/org/bel/birthdeath/death/model/UserSearchRequest.java
lines 1 to 80, the UserSearchRequest DTO duplicates the
common.model.user.UserSearchRequest class. To fix this, remove this redundant
class and replace all its usages with the existing
common.model.user.UserSearchRequest to maintain a single authoritative
definition and avoid code duplication.
| for (String mobile : mobileNumbers) { | ||
| UserSearchRequest searchRequest = new UserSearchRequest(); | ||
| searchRequest.setRequestInfo(requestInfo); | ||
| searchRequest.setTenantId(tenantId.split("\\.")[0]); | ||
| searchRequest.setMobileNumber(mobile); | ||
|
|
||
| StringBuilder uri = new StringBuilder(config.getUserHost()).append(config.getUserSearchEndpoint()); | ||
| UserDetailResponse response = ownerCall(searchRequest, uri); | ||
|
|
||
| if (response != null && response.getUser() != null) { | ||
| allUsers.addAll(response.getUser()); | ||
| } | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Performance concern: Multiple sequential REST calls.
Making individual REST calls for each mobile number can cause significant performance issues. Consider implementing batch user search or caching.
Consider:
- Implementing a batch search endpoint in the user service
- Adding caching for frequently accessed users
- Implementing parallel requests with proper thread pool management
🤖 Prompt for AI Agents
In
birth-death-services/src/main/java/org/bel/birthdeath/common/services/UserService.java
around lines 339 to 351, the code makes sequential REST calls for each mobile
number, causing performance issues. Refactor to either implement a batch user
search endpoint that accepts multiple mobile numbers in one request, or add
caching for frequently accessed users to reduce calls. Alternatively, if batch
search is not feasible, implement parallel requests with controlled thread pool
management to improve throughput without overwhelming the system.
| @SuppressWarnings("rawtypes") | ||
| UserDetailResponse userCall(Object userRequest, StringBuilder uri) { | ||
| String dobFormat = null; | ||
| if (uri.toString().contains(config.getUserSearchEndpoint()) | ||
| || uri.toString().contains(config.getUserUpdateEndpoint())) | ||
| dobFormat = "yyyy-MM-dd"; | ||
| else if (uri.toString().contains(config.getUserCreateEndpoint())) | ||
| dobFormat = "dd/MM/yyyy"; | ||
| try { | ||
|
|
||
| LinkedHashMap responseMap = (LinkedHashMap) serviceRequestRepository.fetchResult(uri, userRequest); | ||
| parseResponse(responseMap, dobFormat); | ||
| return mapper.convertValue(responseMap, UserDetailResponse.class); | ||
| } catch (IllegalArgumentException e) { | ||
| throw new CustomException(Constants.ILLEGAL_ARGUMENT_EXCEPTION, | ||
| "ObjectMapper not able to convertValue in userCall"); | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Eliminate code duplication between userCall and ownerCall.
Both methods have nearly identical implementations. Extract common logic to reduce duplication.
Create a common method:
private UserDetailResponse makeUserServiceCall(Object request, StringBuilder uri) {
String dobFormat = uri.toString().contains(config.getUserSearchEndpoint())
|| uri.toString().contains(config.getUserUpdateEndpoint()) ? "yyyy-MM-dd" : "dd/MM/yyyy";
try {
LinkedHashMap<String, Object> responseMap = (LinkedHashMap<String, Object>) serviceRequestRepository.fetchResult(uri, request);
parseResponse(responseMap, dobFormat);
return mapper.convertValue(responseMap, UserDetailResponse.class);
} catch (IllegalArgumentException e) {
throw new CustomException(Constants.ILLEGAL_ARGUMENT_EXCEPTION,
"ObjectMapper not able to convert response");
}
}Also applies to: 273-289
🤖 Prompt for AI Agents
In
birth-death-services/src/main/java/org/bel/birthdeath/common/services/UserService.java
between lines 194-211 and 273-289, the userCall and ownerCall methods have
nearly identical code. To fix this, extract the shared logic into a single
private method that accepts the request object and URI, determines the dobFormat
based on the URI, fetches and parses the response, and converts it to
UserDetailResponse. Then update both userCall and ownerCall to delegate to this
new method, eliminating duplication.
birth-death-services/src/main/java/org/bel/birthdeath/common/services/UserService.java
Show resolved
Hide resolved
| @Autowired | ||
| CommonConfiguration config; | ||
|
|
||
| @Autowired | ||
| private ServiceRequestRepository serviceRequestRepository; | ||
|
|
||
| @Autowired | ||
| private ObjectMapper mapper; |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Consider using constructor injection for better testability.
Field injection makes the class harder to test. Constructor injection is the recommended approach.
- @Autowired
- CommonConfiguration config;
-
- @Autowired
- private ServiceRequestRepository serviceRequestRepository;
-
- @Autowired
- private ObjectMapper mapper;
+ private final CommonConfiguration config;
+ private final ServiceRequestRepository serviceRequestRepository;
+ private final ObjectMapper mapper;
+
+ @Autowired
+ public UserService(CommonConfiguration config,
+ ServiceRequestRepository serviceRequestRepository,
+ ObjectMapper mapper) {
+ this.config = config;
+ this.serviceRequestRepository = serviceRequestRepository;
+ this.mapper = mapper;
+ }📝 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 | |
| CommonConfiguration config; | |
| @Autowired | |
| private ServiceRequestRepository serviceRequestRepository; | |
| @Autowired | |
| private ObjectMapper mapper; | |
| private final CommonConfiguration config; | |
| private final ServiceRequestRepository serviceRequestRepository; | |
| private final ObjectMapper mapper; | |
| @Autowired | |
| public UserService(CommonConfiguration config, | |
| ServiceRequestRepository serviceRequestRepository, | |
| ObjectMapper mapper) { | |
| this.config = config; | |
| this.serviceRequestRepository = serviceRequestRepository; | |
| this.mapper = mapper; | |
| } |
🤖 Prompt for AI Agents
In
birth-death-services/src/main/java/org/bel/birthdeath/common/services/UserService.java
around lines 37 to 44, replace the current field injection of
CommonConfiguration, ServiceRequestRepository, and ObjectMapper with constructor
injection. Remove the @Autowired annotations from the fields and create a
constructor that takes these dependencies as parameters, assigning them to the
fields. This change will improve testability by allowing easier mocking and
dependency management.
| throw new CustomException("PARSING_ERROR", "Failed to parse dateTolong" + e); | ||
| } | ||
| return d != null ? d.getTime() : 0; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve error message and null date handling.
The error message is unclear and returning 0 for null dates could cause issues.
-} catch (ParseException e) {
- throw new CustomException("PARSING_ERROR", "Failed to parse dateTolong" + e);
-}
-return d != null ? d.getTime() : 0;
+} catch (ParseException e) {
+ throw new CustomException("PARSING_ERROR", "Failed to parse date: " + date + " with format: " + format, e);
+}
+return d != null ? d.getTime() : null;Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
birth-death-services/src/main/java/org/bel/birthdeath/common/services/UserService.java
around lines 314 to 316, improve the error message by making it more descriptive
and clear, such as specifying the context of the parsing failure. Also, instead
of returning 0 when the date is null, handle the null case explicitly, possibly
by throwing an exception or returning an Optional or a sentinel value that
clearly indicates the absence of a valid date, to avoid potential issues from
using 0 as a timestamp.
There was a problem hiding this comment.
Actionable comments posted: 4
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
birth-death-services/src/main/resources/application.properties(6 hunks)
🔇 Additional comments (3)
birth-death-services/src/main/resources/application.properties (3)
3-3: Hard-codingserver.portto 8080 may break containerised / multi-instance setups
Using a fixed port stops Spring from picking an ephemeral port during integration tests and can clash when several services are run locally (all egov micro-services default to 8080). Consider leaving it unset inapplication.propertiesand overriding via an env var or profile-specificapplication-local.properties.
20-20: Flyway enabled without a baseline can brick existing databases
Settingspring.flyway.enabled=trueon an already populated schema will fail unlessbaselineVersion/baselineOnMigrateis configured correctly and an initial baseline script exists. Please verify this against a copy of production before merging.
68-69: Host/port shuffle – double-check inter-service contracts & firewall rules
MDMS → 8084, Billing → 8082, PDF-svc → 8081 diverge from the standard Helm charts (usually 8080). Make sure:
- Ingress rules & service names are updated in deployment manifests.
- Consumers (
birth-deathUI, collection-service workflows) read these values from ConfigMaps, not hard-coded URLs.Also applies to: 72-73, 82-85
| spring.datasource.url=jdbc:postgresql://localhost:5432/postgres | ||
| spring.datasource.username=postgres | ||
| spring.datasource.password=postgres |
There was a problem hiding this comment.
Plain-text credentials & hard-wired JDBC URL introduce security and portability risks
postgres/postgres in VCS + the database name change to postgres can accidentally point every developer to the same DB and leak credentials into lower environments.
Use Spring placeholders (${DB_URL:${DEFAULT_DB_URL}}, ${DB_USER}, ${DB_PASS}) or the Kubernetes secret-based config that the other eGov services follow.
Also applies to: 13-15
🤖 Prompt for AI Agents
In birth-death-services/src/main/resources/application.properties around lines 8
to 10 and 13 to 15, the database credentials and JDBC URL are hardcoded with
plain-text values, which poses security and portability risks. Replace these
hardcoded values with Spring placeholders like ${DB_URL:${DEFAULT_DB_URL}},
${DB_USER}, and ${DB_PASS} to allow external configuration via environment
variables or Kubernetes secrets, following the pattern used in other eGov
services.
| egov.payment.search.endpoint=/collection-services/payments/_search | ||
|
|
||
| spring.main.allow-circular-references=true | ||
| spring.main.allow-circular-references=true |
There was a problem hiding this comment.
🛠️ Refactor suggestion
spring.main.allow-circular-references=true masks a design smell
Circular-dependency suppression hides incorrect bean wiring and was deprecated for removal in Spring 6. Resolve the cycle (likely between UserService, BirthService, and DeathService) instead of enabling this flag.
🤖 Prompt for AI Agents
In birth-death-services/src/main/resources/application.properties at line 121,
the property spring.main.allow-circular-references=true is used to suppress
circular dependencies, which masks underlying design issues and is deprecated in
Spring 6. Remove this property and refactor the code to break the circular
dependency between UserService, BirthService, and DeathService by redesigning
their interactions or using constructor injection with @Lazy annotations or
setter injection to resolve the cycle properly.
|
|
||
| #state level tenant | ||
| egov.state.level.tenant.id=pb | ||
| egov.state.level.tenant.id=pg |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Tenant switch from pb ➜ pg can surface data-visibility bugs
All search, ID-gen formats and MDMS master data now rely on pg. Ensure MDMS has the master data for pg and that existing prod certificates for pb are still reachable (e.g., by retaining pb in egov.bnd.freedownload.tenants).
Also applies to: 108-110
🤖 Prompt for AI Agents
In birth-death-services/src/main/resources/application.properties at lines 97
and 108-110, the tenant ID has been switched from 'pb' to 'pg'. Verify that the
MDMS contains master data for 'pg' and confirm that existing production
certificates for 'pb' remain accessible by keeping 'pb' listed in
egov.bnd.freedownload.tenants. Update these properties accordingly to prevent
data-visibility issues.
| egov.user.host=http://localhost:8085 | ||
| egov.user.search.path=/user/_search | ||
|
|
||
| #User config | ||
| ## changed to egov.user.host | ||
| egov.user.context.path=/user/users | ||
| egov.user.create.path=/_createnovalidate | ||
| egov.user.update.path=/_updatenovalidate | ||
| egov.user.username.prefix=FSM- |
There was a problem hiding this comment.
New User-service paths contain inconsistencies & legacy artefacts
egov.user.context.path=/user/usersduplicates/user– resulting path becomes/user/users/...which is different from the standard/user/_search.egov.user.username.prefix=FSM-is copied from the FSM module and is unrelated to Birth/Death.- The create/update paths use the now-deprecated
_*novalidateendpoints. Switch to/v1/_create&/v1/_updateused in the latest user-service.
Suggested patch:
-egov.user.context.path=/user/users
-egov.user.create.path=/_createnovalidate
-egov.user.update.path=/_updatenovalidate
-egov.user.username.prefix=FSM-
+egov.user.context.path=/user
+egov.user.create.path=/v1/_create
+egov.user.update.path=/v1/_update
+egov.user.username.prefix=BND-📝 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.
| egov.user.host=http://localhost:8085 | |
| egov.user.search.path=/user/_search | |
| #User config | |
| ## changed to egov.user.host | |
| egov.user.context.path=/user/users | |
| egov.user.create.path=/_createnovalidate | |
| egov.user.update.path=/_updatenovalidate | |
| egov.user.username.prefix=FSM- | |
| egov.user.host=http://localhost:8085 | |
| egov.user.search.path=/user/_search | |
| #User config | |
| ## changed to egov.user.host | |
| egov.user.context.path=/user | |
| egov.user.create.path=/v1/_create | |
| egov.user.update.path=/v1/_update | |
| egov.user.username.prefix=BND- |
🤖 Prompt for AI Agents
In birth-death-services/src/main/resources/application.properties around lines
123 to 131, fix the user-service paths by removing the duplicate '/users'
segment from egov.user.context.path so it aligns with the standard '/user' base
path, update egov.user.username.prefix to a relevant prefix for Birth/Death
instead of 'FSM-', and replace the deprecated create and update paths
'/_createnovalidate' and '/_updatenovalidate' with the current '/v1/_create' and
'/v1/_update' endpoints to match the latest user-service API.
CCSD- 370 fix
corrected the pincode,locality dropdown logic in obps ne buildingperm…
salaudeenegov
force-pushed
the
birth-death
branch
from
675f4d9 to
303d7a7
Compare
This reverts commit 303d7a7.
nitish-egov
force-pushed
the
birth-death
branch
from
70ab141 to
7a9e7b4
Compare
Summary by CodeRabbit
New Features
Enhancements
Bug Fixes
Style
Chores