Dev

[holashchand]

Summary by CodeRabbit

Release Notes

New Features

• Added email notification functionality for HRMS module.
• Introduced household type classification (FAMILY, COMMUNITY, OTHER).
• Enhanced project search capabilities with ancestor project ID support.
• Added beneficiary type enum for more structured project management.

Improvements

• Updated dependency versions across multiple services.
• Improved search and filtering mechanisms for projects and households.
• Enhanced validation for household and project-related operations.
• Added more robust type handling for beneficiary and household types.

Bug Fixes

• Resolved issues with project and household search queries.
• Improved data consistency in household and project management.

Technical Updates

• Updated database schemas to support new fields.
• Refactored repository and service layer implementations.
• Enhanced JSON serialization and deserialization processes.

Configuration Changes

• Added new configuration properties for email notifications.
• Updated Kafka topic configurations.
• Introduced new validation settings for household types.

[coderabbitai]

Caution

Review failed

The pull request is closed.

Walkthrough

This pull request encompasses a comprehensive set of changes across multiple services, primarily focusing on the HRMS (Human Resource Management System) and Household modules. The modifications include adding email notification capabilities, introducing household type validation, enhancing project search functionality, and updating various models and configurations. The changes span dependency management, database schema updates, service logic modifications, and the introduction of new validation mechanisms.

Changes

File/Path	Change Summary
core-services/egov-hrms/pom.xml	Updated PostgreSQL dependency version to 42.7.1 and removed Flyway dependency version.
core-services/egov-hrms/src/main/java/org/egov/hrms/config/PropertiesManager.java	Added email notification configuration properties.
core-services/egov-hrms/src/main/resources/application.properties	Added Kafka topics and email notification configurations.
core-services/egov-hrms/src/main/java/org/egov/hrms/consumer/HrmsConsumer.java	Updated Kafka listener to handle new email notification topic.
core-services/egov-hrms/src/main/java/org/egov/hrms/service/EmployeeService.java	Integrated IndividualService and updated create/search methods.
core-services/egov-hrms/src/main/java/org/egov/hrms/service/IndividualService.java	Added method for user creation with locality code.
core-services/egov-hrms/src/main/java/org/egov/hrms/service/NotificationService.java	Introduced method for processing email notifications.
core-services/egov-hrms/src/main/java/org/egov/hrms/utils/HRMSConstants.java	Added new constants for email localization.
core-services/egov-hrms/src/main/java/org/egov/hrms/utils/NotificationUtil.java	Added utility methods for handling email notifications.
core-services/egov-hrms/src/main/java/org/egov/hrms/web/contract/Email.java	Introduced new class to encapsulate email properties.
core-services/egov-hrms/src/main/java/org/egov/hrms/web/contract/EmailRequest.java	Introduced new class for email requests.
core-services/egov-hrms/src/main/java/org/egov/hrms/web/contract/EmployeeResponse.java	Added totalCount field to employee response.
core-services/egov-hrms/src/main/java/org/egov/hrms/web/contract/EmployeeSearchCriteria.java	Updated access modifiers and added userServiceUuids field.
core-services/egov-hrms/src/main/java/org/egov/hrms/web/contract/UserResponse.java	Added totalCount field to user response.
core-services/egov-hrms/src/main/java/org/egov/hrms/web/models/IndividualBulkResponse.java	Introduced new response model for individual bulk operations.
core-services/egov-hrms/src/main/resources/db/migration/main/V20241114114225__household_type_ddl.sql	Added householdType column to HOUSEHOLD table.
health-services/household/src/main/resources/application.properties	Added properties for household type validation and community creator role.
health-services/household/src/main/java/org/egov/household/validators/household/HCommunityTypeValidator.java	Introduced new validator for community type households.
health-services/household/src/main/java/org/egov/household/validators/household/HCommunityValidator.java	Introduced new validator for community households.
health-services/household/src/main/java/org/egov/household/validators/household/HHouseholdTypeChangeValidator.java	Introduced validator for household type changes.
health-services/libraries/health-services-models/src/main/java/org/egov/common/models/household/HouseHoldType.java	Introduced new HouseHoldType enum.
health-services/libraries/health-services-models/src/main/java/org/egov/common/models/project/BeneficiaryType.java	Introduced new BeneficiaryType enum.
health-services/libraries/health-services-models/src/main/java/org/egov/common/models/project/ProjectType.java	Changed beneficiaryType field to BeneficiaryType type.
health-services/libraries/health-services-models/src/main/java/org/egov/common/models/project/Target.java	Changed beneficiaryType field to BeneficiaryType type.
health-services/project/src/main/java/org/egov/project/repository/ProjectRepository.java	Added isAncestorProjectId parameter to project search methods.
health-services/project/src/main/java/org/egov/project/repository/querybuilder/ProjectAddressQueryBuilder.java	Updated query methods to consider ancestor project IDs.
health-services/project/src/main/java/org/egov/project/service/ProjectService.java	Updated search and count methods to handle ancestor project IDs.
health-services/project/src/main/java/org/egov/project/web/controllers/ProjectApiController.java	Updated searchProject method to include ancestor project ID parameter.

Possibly related PRs

• [HCMPRE-857, HCMPRE-1139, HCMPRE-1303, HCMPRE-1158] Changes in hrms-service for email notification feature, UUID and role search changes, Pagination and case insensitive fuzzy search in user name #1337: Updates PostgreSQL dependency version.

---

📜 Recent review details

Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between beca052 and 786d735.

📒 Files selected for processing (2)

• health-services/household/CHANGELOG.md (1 hunks)
• health-services/household/pom.xml (2 hunks)

---

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?

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

• Review comments: Directly reply to a review comment made by CodeRabbit. Example:
  • I pushed a fix in commit <commit_id>, please review it.
  • Generate unit testing code for this file.
  • Open a follow-up GitHub issue for this discussion.
• Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
  • @coderabbitai generate unit testing code for this file.
  • @coderabbitai modularize this function.
• PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
  • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
  • @coderabbitai read src/utils.ts and generate unit testing code.
  • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
  • @coderabbitai help me debug CodeRabbit configuration file.

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)

• @coderabbitai pause to pause the reviews on a PR.
• @coderabbitai resume to resume the paused reviews.
• @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
• @coderabbitai full review to do a full review from scratch and review all the files again.
• @coderabbitai summary to regenerate the summary of the PR.
• @coderabbitai generate docstrings to generate docstrings for this PR. (Beta)
• @coderabbitai resolve resolve all the CodeRabbit review comments.
• @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
• @coderabbitai help to get help.

Other keywords and placeholders

• Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
• Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
• Add @coderabbitai anywhere in the PR title to generate the title automatically.

Documentation and Community

• Visit our Documentation for detailed information on how to use CodeRabbit.
• Join our Discord Community to get help, request features, and share feedback.
• Follow us on X/Twitter for updates and announcements.

[coderabbitai]

Caution

Inline review comments failed to post. This is likely due to GitHub's limits when posting large numbers of comments.

🛑 Comments failed to post (61)

health-services/household/src/main/java/org/egov/household/validators/household/HCommunityValidator.java (3)

22-24: 🧹 Nitpick (assertive)

Unused logger annotation.

The class is annotated with @Slf4j, but there are no log statements. If logging isn’t required, consider removing the @Slf4j annotation to keep the code clean. Otherwise, add log statements where appropriate for traceability.

---

47-57: 🧹 Nitpick (assertive)

Provide detailed context in the error message.

Currently, the error message is generic. Consider enhancing it to include the problematic household’s identifier (e.g., household.getId()) or user details for clarity when troubleshooting access issues.

Error error = Error.builder()
    .errorMessage(String.format("User doesn't have permission to create/update community household. Household id: %s", household.getId()))
    .errorCode("COMMUNITY_USER_ACCESS_DENIED")
    .type(Error.ErrorType.NON_RECOVERABLE)
    .exception(new CustomException("COMMUNITY_USER_ACCESS_DENIED",
        String.format("User doesn't have permission to create/update community household. Household id: %s", household.getId())))
    .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.

            communityHouseholds.forEach(household -> {
                Error error = Error.builder()
                        .errorMessage(String.format("User doesn't have permission to create/update community household. Household id: %s", household.getId()))
                        .errorCode("COMMUNITY_USER_ACCESS_DENIED")
                        .type(Error.ErrorType.NON_RECOVERABLE)
                        .exception(new CustomException("COMMUNITY_USER_ACCESS_DENIED", 
                            String.format("User doesn't have permission to create/update community household. Household id: %s", household.getId())))
                        .build();
                // Populate error details for the household
                populateErrorDetails(household, error, errorDetailsMap);
            });
        }

---

43-46: ⚠️ Potential issue

Potential null-pointer risk on user roles.

request.getRequestInfo().getUserInfo().getRoles() is accessed without null checks. If requestInfo or userInfo is null, this could throw a NullPointerException. Consider adding defensive checks or ensuring that requestInfo and userInfo are never null prior to validator invocation.

+ if (request.getRequestInfo() == null || request.getRequestInfo().getUserInfo() == null) {
+     log.error("Missing user info in request.");
+     // Optionally populate an error object or handle gracefully
+ }

Committable suggestion skipped: line range outside the PR's diff.

core-services/egov-hrms/src/main/java/org/egov/hrms/web/contract/EmployeeResponse.java (1)

73-75: 🧹 Nitpick (assertive)

LGTM! Consider adding documentation.

The field addition follows the existing naming conventions and properly uses @Builder.Default for initialization. Consider adding Javadoc to document the purpose and usage of this field.

Add documentation above the field:

+    /**
+     * Total count of employees returned from the user service.
+     * Defaults to 0 if not explicitly set.
+     */
     @JsonProperty("TotalCount")
     @Builder.Default
     private Long totalCount = 0L;

📝 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.

    /**
     * Total count of employees returned from the user service.
     * Defaults to 0 if not explicitly set.
     */
	@JsonProperty("TotalCount")
	@Builder.Default
	private Long totalCount = 0L;

health-services/libraries/health-services-models/src/main/java/org/egov/common/models/referralmanagement/beneficiarydownsync/DownsyncCLFHouseholdResponse.java (3)

19-20: 🧹 Nitpick (assertive)

Consider following standard JSON naming conventions.

The @JsonProperty annotations use PascalCase ("ResponseInfo", "Households") which is uncommon in JSON. Consider using camelCase for consistency with typical JSON conventions.

-@JsonProperty("ResponseInfo")
+@JsonProperty("responseInfo")
 private ResponseInfo responseInfo;

-@JsonProperty("Households")
+@JsonProperty("households")
-private DownsyncCLFHousehold Households;
+private List<DownsyncCLFHousehold> households;

Also applies to: 22-23

---

12-16: 🧹 Nitpick (assertive)

Consider using more specific Lombok annotations.

Instead of using @Data, consider using a more specific combination of annotations like @Getter and @ToString. The @Data annotation includes @EqualsAndHashCode which might not be necessary for a response model and could potentially cause issues if not properly implemented.

-@Data
+@Getter
+@ToString
 @NoArgsConstructor
 @AllArgsConstructor
 @Builder

📝 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.

@Getter
@ToString
@NoArgsConstructor
@AllArgsConstructor
@Builder

---

22-23: 🛠️ Refactor suggestion

Fix field naming convention and type mismatch.

The Households field has several issues:

1. The field name starts with an uppercase letter, violating Java naming conventions
2. The name suggests a collection, but the type is a single DownsyncCLFHousehold

 @JsonProperty("Households")
-private DownsyncCLFHousehold Households;
+private List<DownsyncCLFHousehold> households;

Committable suggestion skipped: line range outside the PR's diff.

core-services/egov-hrms/src/main/java/org/egov/hrms/web/contract/EmployeeSearchCriteria.java (2)

26-55: 🧹 Nitpick (assertive)

Add size constraints to String fields.

Consider adding @Size constraints to other String fields (codes, names, phone, etc.) to prevent potential issues with extremely large strings. This helps in:

• Validating input data
• Preventing potential performance issues
• Ensuring database column size compatibility

Example implementation:

    @Size(max = 100)
    private List<String> codes;
    
    @Size(max = 100)
    private List<String> names;
    
    @Size(max = 50)
    private List<String> departments;
    
    @Size(max = 50)
    private List<String> designations;
    
    @Size(max = 10)
    private String phone;

---

46-46: 🛠️ Refactor suggestion

Update isCriteriaEmpty method to include the new field.

The newly added userServiceUuids field should be included in the isCriteriaEmpty check to maintain consistency with other fields.

Apply this diff to update the method:

     public boolean isCriteriaEmpty(EmployeeSearchCriteria criteria) {
         if(CollectionUtils.isEmpty(criteria.getCodes()) && CollectionUtils.isEmpty(criteria.getNames()) 
                 && CollectionUtils.isEmpty(criteria.getDepartments()) && CollectionUtils.isEmpty(criteria.getDesignations())
                 && CollectionUtils.isEmpty(criteria.getIds()) && CollectionUtils.isEmpty(criteria.getEmployeestatuses())
                 && CollectionUtils.isEmpty(criteria.getEmployeetypes()) && CollectionUtils.isEmpty(criteria.getUuids())
-                && CollectionUtils.isEmpty(criteria.getPositions()) && StringUtils.isEmpty(criteria.getTenantId())
+                && CollectionUtils.isEmpty(criteria.getPositions()) && CollectionUtils.isEmpty(criteria.getUserServiceUuids())
+                && StringUtils.isEmpty(criteria.getTenantId())
                 && CollectionUtils.isEmpty(criteria.getRoles()) && null == criteria.getAsOnDate()) {
             return true;
         }else {
             return false;
         }
     }

📝 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 boolean isCriteriaEmpty(EmployeeSearchCriteria criteria) {
        if(CollectionUtils.isEmpty(criteria.getCodes()) && CollectionUtils.isEmpty(criteria.getNames()) 
                && CollectionUtils.isEmpty(criteria.getDepartments()) && CollectionUtils.isEmpty(criteria.getDesignations())
                && CollectionUtils.isEmpty(criteria.getIds()) && CollectionUtils.isEmpty(criteria.getEmployeestatuses())
                && CollectionUtils.isEmpty(criteria.getEmployeetypes()) && CollectionUtils.isEmpty(criteria.getUuids())
                && CollectionUtils.isEmpty(criteria.getPositions()) && CollectionUtils.isEmpty(criteria.getUserServiceUuids())
                && StringUtils.isEmpty(criteria.getTenantId())
                && CollectionUtils.isEmpty(criteria.getRoles()) && null == criteria.getAsOnDate()) {
            return true;
        }else {
            return false;
        }
    }

health-services/libraries/health-services-models/src/main/java/org/egov/common/models/referralmanagement/beneficiarydownsync/HouseholdMemberMap.java (1)

13-20: 🛠️ Refactor suggestion

Add field validation and documentation.

The class lacks proper validation and documentation. Consider the following improvements:

1. Add JSR-303 validation annotations
2. Add Javadoc explaining the class purpose and field usage

+import javax.validation.constraints.NotNull;
+import javax.validation.constraints.Min;
+
+/**
+ * Maps a household to its member count for beneficiary downsync operations.
+ */
 @Builder
 public class HouseholdMemberMap {
 
+    @NotNull(message = "Household cannot be null")
     private Household household;
 
+    @NotNull(message = "Number of members cannot be null")
+    @Min(value = 0, message = "Number of members cannot be negative")
     private Integer numberOfMembers;
 }

📝 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 javax.validation.constraints.NotNull;
import javax.validation.constraints.Min;

/**
 * Maps a household to its member count for beneficiary downsync operations.
 */
@Builder
public class HouseholdMemberMap {

    @NotNull(message = "Household cannot be null")
    private Household household;

    @NotNull(message = "Number of members cannot be null")
    @Min(value = 0, message = "Number of members cannot be negative")
    private Integer numberOfMembers;

}

health-services/household/src/main/java/org/egov/household/validators/household/HCommunityTypeValidator.java (1)

27-52: 🛠️ Refactor suggestion

Optimize validation logic and error handling.

Several improvements can be made to enhance the code:

1. Avoid duplicating the error message
2. Create a single error for the validation failure
3. Make the error message more descriptive
4. Simplify the stream operation

Here's the suggested implementation:

     @Override
     public Map<Household, List<Error>> validate(HouseholdBulkRequest request) {
         HashMap<Household, List<Error>> errorDetailsMap = new HashMap<>();
         if (configuration.isHouseholdTypeSameValidation()) {
-            // validate if request contains households of different householdTypes
+            // Count community households to validate mixed types
             List<Household> communityHouseholds = request.getHouseholds()
                     .stream()
-                    .filter(household -> household.getHouseholdType() != null &&
-                            household.getHouseholdType().equals(HouseHoldType.COMMUNITY))
+                    .filter(household -> HouseHoldType.COMMUNITY.equals(household.getHouseholdType()))
                     .toList();

             if (!CollectionUtils.isEmpty(communityHouseholds) &&
                     request.getHouseholds().size() != communityHouseholds.size()) {
+                String errorMessage = "Mixed household types detected. Community households must be processed separately from Family households.";
+                Error error = Error.builder()
+                        .errorMessage(errorMessage)
+                        .errorCode("MIXED_HOUSEHOLD_TYPES_NOT_ALLOWED")
+                        .type(Error.ErrorType.NON_RECOVERABLE)
+                        .exception(new CustomException("MIXED_HOUSEHOLD_TYPES_NOT_ALLOWED", errorMessage))
+                        .build();
+
                 communityHouseholds.forEach(household -> {
-                    Error error = Error.builder()
-                            .errorMessage("Community and Family household cannot be in same request")
-                            .errorCode("COMMUNITY_AND_FAMILY_HOUSEHOLD_IN_SAME_REQUEST")
-                            .type(Error.ErrorType.NON_RECOVERABLE)
-                            .exception(new CustomException("COMMUNITY_AND_FAMILY_HOUSEHOLD_IN_SAME_REQUEST", "Community and Family household cannot be in same request"))
-                            .build();
-                    // Populate error details for the household
                     populateErrorDetails(household, error, errorDetailsMap);
                 });
             }
         }
         return errorDetailsMap;
     }

The changes:

1. Simplified the null check by moving HouseHoldType.COMMUNITY to the left side of equals
2. Created a single Error object outside the loop
3. Made the error message more descriptive
4. Improved the error code to better reflect the validation rule

📝 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 Map<Household, List<Error>> validate(HouseholdBulkRequest request) {
        HashMap<Household, List<Error>> errorDetailsMap = new HashMap<>();
        if (configuration.isHouseholdTypeSameValidation()) {
            // Count community households to validate mixed types
            List<Household> communityHouseholds = request.getHouseholds()
                    .stream()
                    .filter(household -> HouseHoldType.COMMUNITY.equals(household.getHouseholdType()))
                    .toList();

            if (!CollectionUtils.isEmpty(communityHouseholds) &&
                    request.getHouseholds().size() != communityHouseholds.size()) {
                String errorMessage = "Mixed household types detected. Community households must be processed separately from Family households.";
                Error error = Error.builder()
                        .errorMessage(errorMessage)
                        .errorCode("MIXED_HOUSEHOLD_TYPES_NOT_ALLOWED")
                        .type(Error.ErrorType.NON_RECOVERABLE)
                        .exception(new CustomException("MIXED_HOUSEHOLD_TYPES_NOT_ALLOWED", errorMessage))
                        .build();

                communityHouseholds.forEach(household -> {
                    populateErrorDetails(household, error, errorDetailsMap);
                });
            }
        }
        return errorDetailsMap;
    }

health-services/libraries/health-services-models/src/main/java/org/egov/common/models/referralmanagement/beneficiarydownsync/DownsyncCLFHousehold.java (3)

8-8: 🧹 Nitpick (assertive)

Remove unused imports.

The following imports are not used in the code:

• org.egov.common.models.household.Household
• java.util.Map

-import org.egov.common.models.household.Household;
 
 import java.util.List;
-import java.util.Map;

Also applies to: 11-12

---

22-23: 🧹 Nitpick (assertive)

Standardize JSON property naming convention.

Consider using camelCase for JSON property names to follow standard JSON naming conventions.

-	@JsonProperty("DownsyncCriteria")
+	@JsonProperty("downsyncCriteria")
+	@NotNull
 	DownsyncCriteria downsyncCriteria;

📝 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("downsyncCriteria")
	@NotNull
	DownsyncCriteria downsyncCriteria;

---

19-20: 🧹 Nitpick (assertive)

Fix property name mismatch and add validation.

The JSON property name "HouseholdCountMap" doesn't match the variable name "householdMemberCountMap". Consider:

1. Aligning the names for better clarity
2. Adding validation annotations for robust data handling

-	@JsonProperty("HouseholdCountMap")
+	@JsonProperty("householdMemberCountMap")
+	@NotNull
 	List<HouseholdMemberMap> householdMemberCountMap;

📝 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("householdMemberCountMap")
	@NotNull
	List<HouseholdMemberMap> householdMemberCountMap;

core-services/egov-hrms/src/main/java/org/egov/hrms/utils/HRMSConstants.java (2)

44-44: 🧹 Nitpick (assertive)

Fix typo in constant name: "CRITERA" should be "CRITERIA".

The constant name contains a typo: HRMS_USER_SEARCH_CRITERA_USER_SERVICE_UUIDS should be HRMS_USER_SEARCH_CRITERIA_USER_SERVICE_UUIDS.

-public static final String HRMS_USER_SEARCH_CRITERA_USER_SERVICE_UUIDS = "userServiceUuids";
+public static final String HRMS_USER_SEARCH_CRITERIA_USER_SERVICE_UUIDS = "userServiceUuids";

📝 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 static final String HRMS_USER_SEARCH_CRITERIA_USER_SERVICE_UUIDS = "userServiceUuids";

---

34-34: 💡 Codebase verification

Confirmed: Localization code uses inconsistent casing convention

The constant HEALTH_HRMS_EMAIL_LOCALIZATION_CODE with value "HEALTH_HRMS_EMAIL_CODE" is inconsistent with other localization codes in the file which use kebab-case with lowercase letters and dots, such as:

• "hrms.employee.create.notification"
• "hrms.employee.reactivation.notification"

Consider changing it to follow the same convention, e.g., "health.hrms.email.code".

🔗 Analysis chain

Consider using consistent casing convention for localization code.

The constant HEALTH_HRMS_EMAIL_LOCALIZATION_CODE uses snake_case for its value while other localization codes in the file use kebab-case. This inconsistency could lead to issues if the actual localization key doesn't match.

Run this script to check the casing convention used in the localization service:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check the casing convention used in the localization service for the rainmaker-hr module

curl -s "https://dev.digit.org/localization/messages/v1/_search?module=rainmaker-hr&locale=en_IN" | jq -r '.messages[] | select(.code | contains("EMAIL")) | .code'

Length of output: 165

---

Script:

#!/bin/bash
# Check other constants in the same file and similar patterns across codebase
echo "=== Constants in HRMSConstants.java ==="
ast-grep --pattern 'public static final String $_ = "$_";'  core-services/egov-hrms/src/main/java/org/egov/hrms/utils/HRMSConstants.java

echo -e "\n=== Searching for localization codes across codebase ==="
rg -l "localization|localisation" --type yaml

echo -e "\n=== Checking for other email related codes ==="
rg "EMAIL.*CODE" --type java

Length of output: 1296

---

Script:

#!/bin/bash
# Check for localization related constants and their values
echo "=== Content of HRMSConstants.java ==="
cat core-services/egov-hrms/src/main/java/org/egov/hrms/utils/HRMSConstants.java

echo -e "\n=== Searching for localization message patterns ==="
rg "messages.*localization|localizationMessages" --type java -A 2 -B 2

Length of output: 5730

health-services/libraries/health-services-models/src/main/java/org/egov/common/models/referralmanagement/beneficiarydownsync/DownsyncCriteria.java (1)

38-38: 🧹 Nitpick (assertive)

Remove extra blank line.

Remove the unnecessary blank line to maintain consistent code style.

 	private Long totalCount;
-

 	private String householdId;

Committable suggestion skipped: line range outside the PR's diff.

core-services/egov-hrms/src/main/java/org/egov/hrms/web/models/IndividualBulkResponse.java (3)

22-29: 🧹 Nitpick (assertive)

Remove extra blank line.

The annotations are well-chosen for a response DTO. Consider removing the extra blank line at line 24 for better readability.

---

46-52: 🧹 Nitpick (assertive)

Consider modernizing the helper method implementation.

The method could be more concise using Java 8+ features and have a more consistent name.

-    public IndividualBulkResponse addIndividualItem(Individual individualItem) {
-        if (this.individual == null) {
-            this.individual = new ArrayList<>();
-        }
-        this.individual.add(individualItem);
-        return this;
-    }
+    public IndividualBulkResponse addIndividual(Individual individual) {
+        this.individual = this.individual == null ? new ArrayList<>() : this.individual;
+        this.individual.add(individual);
+        return this;
+    }

📝 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 IndividualBulkResponse addIndividual(Individual individual) {
        this.individual = this.individual == null ? new ArrayList<>() : this.individual;
        this.individual.add(individual);
        return this;
    }

---

32-44: 🧹 Nitpick (assertive)

Standardize JSON property names to follow camelCase convention.

The @JsonProperty annotations use PascalCase ("ResponseInfo", "TotalCount", "Individual") which is inconsistent with typical JSON naming conventions. Consider using camelCase for consistency with standard Java/JSON practices.

-    @JsonProperty("ResponseInfo")
+    @JsonProperty("responseInfo")
     @NotNull
     @Valid
     private ResponseInfo responseInfo = null;

-    @JsonProperty("TotalCount")
+    @JsonProperty("totalCount")
     @Valid
     @Builder.Default
     private Long totalCount = 0L;

-    @JsonProperty("Individual")
+    @JsonProperty("individual")
     @Valid
     private List<Individual> individual = null;

📝 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("responseInfo")
    @NotNull
    @Valid
    private ResponseInfo responseInfo = null;

    @JsonProperty("totalCount")
    @Valid
    @Builder.Default
    private Long totalCount = 0L;

    @JsonProperty("individual")
    @Valid
    private List<Individual> individual = null;

core-services/egov-hrms/src/main/java/org/egov/hrms/service/EmployeeService.java (2)

205-229: 🧹 Nitpick (assertive)

Optimize search criteria building.

The code block for handling userServiceUuids search criteria contains duplicated logic from the previous name search block. Consider extracting the common logic into a helper method.

+private void enrichSearchCriteriaWithUserResponse(EmployeeSearchCriteria criteria, Map<String, Object> userSearchCriteria, 
+        Map<String, User> mapOfUsers, UserResponse userResponse) {
+    List<String> userUUIDs = new ArrayList<>();
+    if(!CollectionUtils.isEmpty(userResponse.getUser())) {
+        mapOfUsers.putAll(userResponse.getUser().stream()
+                .collect(Collectors.toMap(User::getUuid, Function.identity())));
+    }
+    List<String> uuids = userResponse.getUser().stream().map(User :: getUuid).collect(Collectors.toList());
+    userUUIDs.addAll(uuids);
+    
+    if(!CollectionUtils.isEmpty(criteria.getUuids()))
+        criteria.setUuids(criteria.getUuids().stream().filter(userUUIDs::contains).collect(Collectors.toList()));
+    else
+        criteria.setUuids(userUUIDs);
+}

Committable suggestion skipped: line range outside the PR's diff.

---

272-279: 🛠️ Refactor suggestion

Review conditional user creation logic.

The implementation introduces a runtime type check using instanceof, which is generally considered a code smell. Consider using dependency injection or strategy pattern instead.

-if(userService instanceof IndividualService) {
-    String localityCode = (employee.getJurisdictions()!=null && !employee.getJurisdictions().isEmpty())? employee.getJurisdictions().get(0).getBoundary() : null;
-    response = individualService.createUserByLocality(request, localityCode);
-}
-else{
-    response = userService.createUser(request);
-}
+String localityCode = (employee.getJurisdictions()!=null && !employee.getJurisdictions().isEmpty())? 
+    employee.getJurisdictions().get(0).getBoundary() : null;
+response = userService.createUser(request, localityCode);

Committable suggestion skipped: line range outside the PR's diff.

health-services/project/src/main/java/org/egov/project/repository/ProjectRepository.java (1)

84-86: 🧹 Nitpick (assertive)

Consider lazy initialization for collections.
Targets and documents lists are initialized regardless of whether they are used. This is not a big issue, but you could optionally initialize them after you confirm !projectIds.isEmpty().

health-services/project/src/main/java/org/egov/project/service/ProjectService.java (3)

135-135: 🧹 Nitpick (assertive)

Hard-coded false for isAncestorProjectId.
You might later consider making this parameter dynamic. Currently, it’s always false for updates, which may be correct, but keep in mind any evolving requirements.

---

290-291: 🧹 Nitpick (assertive)

Same note on hard-coded false.
Make sure it’s intentional to ignore ancestor references in this cascading date update scenario.

---

312-312: 🧹 Nitpick (assertive)

Parent project lookups always exclude ancestry logic.
If the design changes, consider whether the parent search might need to include ancestor IDs in the future.

health-services/project/CHANGELOG.md (2)

3-5: 🧹 Nitpick (assertive)

Enhance changelog entry with more details.

Consider expanding the changelog entry to provide more context about the feature's impact and usage. For example:

## 1.1.6 - 2025-01-27

-Added isAncestorProjectId param for search projects API to support search projects with ancestor project id as well
+### Added
+- New query parameter `isAncestorProjectId` to the search projects API
+  - When set to true, enables searching projects using an ancestor project ID
+  - Enhances project hierarchy navigation capabilities
+  - Affects `/project/v1/_search` endpoint

📝 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.

## 1.1.6 - 2025-01-27

### Added
- New query parameter `isAncestorProjectId` to the search projects API
  - When set to true, enables searching projects using an ancestor project ID
  - Enhances project hierarchy navigation capabilities
  - Affects `/project/v1/_search` endpoint

🧰 Tools

🪛 markdownlint-cli2 (0.17.2)

3-3: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below

(MD022, blanks-around-headings)

---

4-4: Lists should be surrounded by blank lines
null

(MD032, blanks-around-lists)

---

3-4: 🧹 Nitpick (assertive)

Fix markdown formatting.

Add blank lines around headings and lists to improve readability:

All notable changes to this module will be documented in this file.

+
## 1.1.6 - 2025-01-27
+
- Added isAncestorProjectId param for search projects API to support search projects with ancestor project id as well
+

📝 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.

All notable changes to this module will be documented in this file.

## 1.1.6 - 2025-01-27

- Added isAncestorProjectId param for search projects API to support search projects with ancestor project id as well

🧰 Tools

🪛 markdownlint-cli2 (0.17.2)

3-3: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below

(MD022, blanks-around-headings)

---

4-4: Lists should be surrounded by blank lines
null

(MD032, blanks-around-lists)

health-services/household/src/main/java/org/egov/household/validators/household/HHouseholdTypeChangeValidator.java (4)

72-77: 🧹 Nitpick (assertive)

Clarify or refine error messages.
The error code and message both say "Household Type change," which does not explicitly convey the cause or resolution. Consider enhancing the error message to indicate the violation and potential next steps.

---

84-96: ⚠️ Potential issue

Avoid potential NullPointerExceptions when comparing household types.
If existingEntity.getHouseholdType() or eMap.get(existingEntity.getId()).getHouseholdType() is null, .equals() throws an NPE. Use a null-safe check (e.g., Objects.equals()).

- if (!existingEntity.getHouseholdType()
-     .equals(eMap.get(existingEntity.getId()).getHouseholdType())) {
+ if (!Objects.equals(existingEntity.getHouseholdType(),
+     eMap.get(existingEntity.getId()).getHouseholdType())) {

📝 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 List<Household> changeInHouseholdType(Map<String, Household> eMap,
            List<Household> existingEntities) {
        List<Household> entitiesWithHouseholdTypeChange = new ArrayList<>();

        for (Household existingEntity : existingEntities) {
            if (eMap.containsKey(existingEntity.getId())) {
                if (!Objects.equals(existingEntity.getHouseholdType(),
                    eMap.get(existingEntity.getId()).getHouseholdType())) {
                    entitiesWithHouseholdTypeChange.add(eMap.get(existingEntity.getId()));
                }
            }
        }
        return entitiesWithHouseholdTypeChange;
    }

---

35-39: 🛠️ Refactor suggestion

Handle null or empty request.getHouseholds().
If request.getHouseholds() is null or empty, subsequent operations (e.g., entities.get(0) in line 62) may result in IndexOutOfBoundsException. Consider adding a preliminary null or size check to gracefully handle this scenario.

 public Map<Household, List<Error>> validate(HouseholdBulkRequest request) {
-    List<Household> entities = request.getHouseholds();
+    List<Household> entities = request.getHouseholds() != null ? request.getHouseholds() : Collections.emptyList();
+    if (entities.isEmpty()) {
+        return Collections.emptyMap();
+    }
    ...
 }

📝 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 Map<Household, List<Error>> validate(HouseholdBulkRequest request) {
        // Map to hold household entities and their error details
        Map<Household, List<Error>> errorDetailsMap = new HashMap<>();
        // Get the list of household entities from the request
        List<Household> entities = request.getHouseholds() != null ? request.getHouseholds() : Collections.emptyList();
        if (entities.isEmpty()) {
            return Collections.emptyMap();
        }

---

61-62: ⚠️ Potential issue

Guard against IndexOutOfBoundsException.
entities.get(0) can fail if entities is empty. Ensure that the code checks for emptiness before accessing the first element.

- existingEntities = householdRepository.find(householdSearch, entities.size(), 0,
-       entities.get(0).getTenantId(), null, false).getResponse();
+ String tenantId = entities.get(0).getTenantId();
+ existingEntities = householdRepository.find(householdSearch, entities.size(), 0,
+       tenantId, null, false).getResponse();

Committable suggestion skipped: line range outside the PR's diff.

health-services/project/src/main/java/org/egov/project/repository/querybuilder/ProjectAddressQueryBuilder.java (1)

49-53: 🧹 Nitpick (assertive)

Enhance method documentation for better clarity.

While the documentation explains the purpose of isAncestorProjectId, it would be helpful to include:

• The impact on query behavior
• Example scenarios when this flag should be true/false

 /**
  * Constructs project search query based on conditions
- * @param isAncestorProjectId if set to true, project id in the projects would be considered as ancestor project id.
+ * @param isAncestorProjectId if set to true, matches projects where the given project ID is either:
+ *                           - directly equal to the project's ID, or
+ *                           - present in the project's hierarchy (ancestor relationship)
+ *                           Use this when searching for a project and all its descendants.
  */

📝 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.

    /**
     * Constructs project search query based on conditions
     * @param isAncestorProjectId if set to true, matches projects where the given project ID is either:
     *                           - directly equal to the project's ID, or
     *                           - present in the project's hierarchy (ancestor relationship)
     *                           Use this when searching for a project and all its descendants.
     */
    public String getProjectSearchQuery(List<Project> projects, Integer limit, Integer offset, String tenantId, Long lastChangedSince, Boolean includeDeleted, Long createdFrom, Long createdTo, boolean isAncestorProjectId, List<Object> preparedStmtList, boolean isCountQuery) {

core-services/egov-hrms/src/main/java/org/egov/hrms/utils/NotificationUtil.java (4)

140-153: 🧹 Nitpick (assertive)

Enhance error handling and consider batch processing.

The email sending implementation could be improved:

1. Add error handling for producer failures
2. Include more detailed logging
3. Consider batch processing for better performance

Consider this enhanced implementation:

 public void sendEmail(List<EmailRequest> emailRequestList) {
     if (propertiesManager.getIsEmailNotificationEnabled()) {
         if (CollectionUtils.isEmpty(emailRequestList)) {
             log.error("No Emails Found!");
             return;
         }
+        int batchSize = 10;
+        List<EmailRequest> batch = new ArrayList<>(batchSize);
         
         for (EmailRequest emailRequest : emailRequestList) {
-            producer.push(propertiesManager.getEmailNotifTopic(), emailRequest);
-            log.info("Email Request -> " + emailRequest.toString());
-            log.info("EMAIL notification sent!");
+            try {
+                batch.add(emailRequest);
+                if (batch.size() >= batchSize) {
+                    processBatch(batch);
+                    batch.clear();
+                }
+            } catch (Exception e) {
+                log.error("Failed to process email request: " + emailRequest.toString(), e);
+            }
         }
+        
+        if (!batch.isEmpty()) {
+            processBatch(batch);
+        }
     }
 }

+private void processBatch(List<EmailRequest> batch) {
+    log.info("Processing batch of {} emails", batch.size());
+    try {
+        for (EmailRequest emailRequest : batch) {
+            producer.push(propertiesManager.getEmailNotifTopic(), emailRequest);
+            log.debug("Email queued for delivery: {}", emailRequest.toString());
+        }
+        log.info("Successfully processed batch of {} emails", batch.size());
+    } catch (Exception e) {
+        log.error("Failed to process email batch", e);
+        throw e;
+    }
+}

Committable suggestion skipped: line range outside the PR's diff.

---

49-65: 🧹 Nitpick (assertive)

Add input validation and improve error handling.

The message template extraction could be more robust:

1. Add null checks for input parameters
2. Consider throwing a specific exception instead of returning null
3. Use more specific exception handling

Consider this implementation:

 public String getMessageTemplate(String notificationCode, String localizationMessage) {
+    if (StringUtils.isEmpty(notificationCode) || StringUtils.isEmpty(localizationMessage)) {
+        throw new IllegalArgumentException("Notification code and localization message must not be empty");
+    }
     String path = "$..messages[?(@.code==\"{}\")].message";
     path = path.replace("{}", notificationCode);
-    String message = null;
+    String message;
     try {
         List data = JsonPath.parse(localizationMessage).read(path);
         if (!CollectionUtils.isEmpty(data))
             message = data.get(0).toString();
         else {
             log.error("Fetching from localization failed with code " + notificationCode);
-            return null;
+            throw new IllegalStateException("Message not found for code: " + notificationCode);
         }
-    } catch (Exception e) {
+    } catch (JsonPathException e) {
         log.warn("Fetching from localization failed", e);
-        return null;
+        throw new IllegalStateException("Failed to parse localization message", e);
     }
     return message;
 }

📝 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 String getMessageTemplate(String notificationCode, String localizationMessage) {
        if (StringUtils.isEmpty(notificationCode) || StringUtils.isEmpty(localizationMessage)) {
            throw new IllegalArgumentException("Notification code and localization message must not be empty");
        }
        String path = "$..messages[?(@.code==\"{}\")].message";
        path = path.replace("{}", notificationCode);
        String message;
        try {
            List data = JsonPath.parse(localizationMessage).read(path);
            if (!CollectionUtils.isEmpty(data))
                message = data.get(0).toString();
            else {
                log.error("Fetching from localization failed with code " + notificationCode);
                throw new IllegalStateException("Message not found for code: " + notificationCode);
            }
        } catch (JsonPathException e) {
            log.warn("Fetching from localization failed", e);
            throw new IllegalStateException("Failed to parse localization message", e);
        }
        return message;
    }

---

110-133: ⚠️ Potential issue

Replace unsafe HTML parsing with proper HTML processing.

The current implementation has several issues:

1. Unsafe HTML parsing using string indices
2. No validation of email template format
3. Missing escaping of replacement values

Consider using an HTML parser library and proper template engine:

+import org.jsoup.Jsoup;
+import org.jsoup.nodes.Document;
+import org.apache.commons.text.StringSubstitutor;

 public List<EmailRequest> createEmailRequest(EmployeeRequest employeeRequest, String emailTemplate) {
     RequestInfo requestInfo = employeeRequest.getRequestInfo();
     List<EmailRequest> emailRequest = new LinkedList<>();

     for (Employee employee : employeeRequest.getEmployees()) {
-        String customizedMsg = emailTemplate.replace("{User's name}", employee.getUser().getName());
-        customizedMsg = customizedMsg.replace("{Username}", employee.getCode());
-        customizedMsg = customizedMsg.replace("{Password}", employee.getUser().getPassword());
-        customizedMsg = customizedMsg.replace("{website URL}", propertiesManager.getEmailNotificationWebsiteLink());
-        customizedMsg = customizedMsg.replace("{Implementation partner}", propertiesManager.getEmailNotificationImplementationPartner());
+        Map<String, String> replacements = new HashMap<>();
+        replacements.put("User's name", StringEscapeUtils.escapeHtml4(employee.getUser().getName()));
+        replacements.put("Username", StringEscapeUtils.escapeHtml4(employee.getCode()));
+        replacements.put("Password", StringEscapeUtils.escapeHtml4(employee.getUser().getPassword()));
+        replacements.put("website URL", StringEscapeUtils.escapeHtml4(propertiesManager.getEmailNotificationWebsiteLink()));
+        replacements.put("Implementation partner", StringEscapeUtils.escapeHtml4(propertiesManager.getEmailNotificationImplementationPartner()));
+        
+        StringSubstitutor sub = new StringSubstitutor(replacements, "{", "}");
+        String customizedMsg = sub.replace(emailTemplate);

-        String subject = customizedMsg.substring(customizedMsg.indexOf("<h2>")+4, customizedMsg.indexOf("</h2>"));
-        String body = customizedMsg.substring(customizedMsg.indexOf("</h2>")+5);
+        Document doc = Jsoup.parse(customizedMsg);
+        String subject = doc.select("h2").first().text();
+        doc.select("h2").remove();
+        String body = doc.body().html();

         Email emailObj = Email.builder()
             .emailTo(Collections.singleton(employee.getUser().getEmailId()))
             .isHTML(true)
             .body(body)
             .subject(subject)
             .build();
         EmailRequest email = new EmailRequest(requestInfo, emailObj);
         emailRequest.add(email);
     }
     return emailRequest;
 }

📝 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 List<EmailRequest> createEmailRequest(EmployeeRequest employeeRequest, String emailTemplate) {
        RequestInfo requestInfo = employeeRequest.getRequestInfo();
        List<EmailRequest> emailRequest = new LinkedList<>();

        for (Employee employee : employeeRequest.getEmployees()) {
            Map<String, String> replacements = new HashMap<>();
            replacements.put("User's name", StringEscapeUtils.escapeHtml4(employee.getUser().getName()));
            replacements.put("Username", StringEscapeUtils.escapeHtml4(employee.getCode()));
            replacements.put("Password", StringEscapeUtils.escapeHtml4(employee.getUser().getPassword()));
            replacements.put("website URL", StringEscapeUtils.escapeHtml4(propertiesManager.getEmailNotificationWebsiteLink()));
            replacements.put("Implementation partner", StringEscapeUtils.escapeHtml4(propertiesManager.getEmailNotificationImplementationPartner()));
            
            StringSubstitutor sub = new StringSubstitutor(replacements, "{", "}");
            String customizedMsg = sub.replace(emailTemplate);

            Document doc = Jsoup.parse(customizedMsg);
            String subject = doc.select("h2").first().text();
            doc.select("h2").remove();
            String body = doc.body().html();

            Email emailObj = Email.builder()
                .emailTo(Collections.singleton(employee.getUser().getEmailId()))
                .isHTML(true)
                .body(body)
                .subject(subject)
                .build();
            EmailRequest email = new EmailRequest(requestInfo, emailObj);
            emailRequest.add(email);
        }
        return emailRequest;
    }

---

88-100: 🛠️ Refactor suggestion

Add safety checks for array access and improve locale handling.

The URI construction has potential null pointer and array index issues:

1. Missing null/empty check for employees list
2. Unsafe array access for tenant ID split
3. Complex locale extraction logic

Consider this safer implementation:

 public StringBuilder getUri(EmployeeRequest employeeRequest) {
+    if (employeeRequest == null || CollectionUtils.isEmpty(employeeRequest.getEmployees())) {
+        throw new IllegalArgumentException("Employee request or employees list cannot be empty");
+    }
+    
+    String tenantId = employeeRequest.getEmployees().get(0).getTenantId();
+    if (StringUtils.isEmpty(tenantId)) {
+        throw new IllegalArgumentException("Tenant ID cannot be empty");
+    }
-    String tenantId = employeeRequest.getEmployees().get(0).getTenantId().split("\\.")[0];
+    String[] tenantParts = tenantId.split("\\.");
+    String rootTenantId = tenantParts.length > 0 ? tenantParts[0] : tenantId;

     String locale = HRMS_LOCALIZATION_ENG_LOCALE_CODE;
-    if (!StringUtils.isEmpty(employeeRequest.getRequestInfo().getMsgId()) && employeeRequest.getRequestInfo().getMsgId().split("|").length >= 2)
-        locale = employeeRequest.getRequestInfo().getMsgId().split("\\|")[1];
+    String msgId = employeeRequest.getRequestInfo().getMsgId();
+    if (!StringUtils.isEmpty(msgId)) {
+        String[] msgParts = msgId.split("\\|");
+        if (msgParts.length >= 2) {
+            locale = msgParts[1];
+        }
+    }

     StringBuilder uri = new StringBuilder();
     uri.append(propertiesManager.getLocalizationHost())
        .append(propertiesManager.getLocalizationSearchEndpoint())
        .append("?")
        .append("locale=").append(locale)
-       .append("&tenantId=").append(tenantId)
+       .append("&tenantId=").append(rootTenantId)
        .append("&module=").append(HEALTH_HRMS_LOCALIZATION_MODULE_CODE);

     return uri;
 }

📝 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 StringBuilder getUri(EmployeeRequest employeeRequest) {
        if (employeeRequest == null || CollectionUtils.isEmpty(employeeRequest.getEmployees())) {
            throw new IllegalArgumentException("Employee request or employees list cannot be empty");
        }
        
        String tenantId = employeeRequest.getEmployees().get(0).getTenantId();
        if (StringUtils.isEmpty(tenantId)) {
            throw new IllegalArgumentException("Tenant ID cannot be empty");
        }
        String[] tenantParts = tenantId.split("\\.");
        String rootTenantId = tenantParts.length > 0 ? tenantParts[0] : tenantId;

        String locale = HRMS_LOCALIZATION_ENG_LOCALE_CODE;
        String msgId = employeeRequest.getRequestInfo().getMsgId();
        if (!StringUtils.isEmpty(msgId)) {
            String[] msgParts = msgId.split("\\|");
            if (msgParts.length >= 2) {
                locale = msgParts[1];
            }
        }

        StringBuilder uri = new StringBuilder();
        uri.append(propertiesManager.getLocalizationHost())
           .append(propertiesManager.getLocalizationSearchEndpoint())
           .append("?")
           .append("locale=").append(locale)
           .append("&tenantId=").append(rootTenantId)
           .append("&module=").append(HEALTH_HRMS_LOCALIZATION_MODULE_CODE);

        return uri;
    }

core-services/egov-hrms/src/main/java/org/egov/hrms/service/IndividualService.java (2)

66-79: 🛠️ Refactor suggestion

Reduce code duplication by refactoring createUser and createUserByLocality.

The new method duplicates most of the logic from createUser. Consider refactoring to eliminate this duplication.

Here's a suggested implementation:

-    @Override
-    public UserResponse createUser(UserRequest userRequest) {
-        IndividualRequest request = mapToIndividualRequest(userRequest, null);
-        StringBuilder uri = new StringBuilder();
-        uri.append(propertiesManager.getIndividualHost());
-        uri.append(propertiesManager.getIndividualCreateEndpoint());
-        IndividualResponse response = restCallRepository
-                .fetchResult(uri, request, IndividualResponse.class);
-        UserResponse userResponse = null;
-        if (response != null && response.getIndividual() != null) {
-            log.info("response received from individual service");
-            userResponse = mapToUserResponse(response);
-        }
-        return userResponse;
-    }
-
-    public UserResponse createUserByLocality(UserRequest userRequest, String localityCode) {
-        IndividualRequest request = mapToIndividualRequest(userRequest,localityCode);
-        StringBuilder uri = new StringBuilder();
-        uri.append(propertiesManager.getIndividualHost());
-        uri.append(propertiesManager.getIndividualCreateEndpoint());
-        IndividualResponse response = restCallRepository
-          .fetchResult(uri, request, IndividualResponse.class);
-        UserResponse userResponse = null;
-        if (response != null && response.getIndividual() != null) {
-            log.info("response received from individual service");
-            userResponse = mapToUserResponse(response);
-        }
-        return userResponse;
-    }

+    private UserResponse createUserInternal(UserRequest userRequest, String localityCode) {
+        IndividualRequest request = mapToIndividualRequest(userRequest, localityCode);
+        StringBuilder uri = new StringBuilder()
+                .append(propertiesManager.getIndividualHost())
+                .append(propertiesManager.getIndividualCreateEndpoint());
+        IndividualResponse response = restCallRepository
+                .fetchResult(uri, request, IndividualResponse.class);
+        UserResponse userResponse = null;
+        if (response != null && response.getIndividual() != null) {
+            log.info("response received from individual service");
+            userResponse = mapToUserResponse(response);
+        }
+        return userResponse;
+    }
+
+    @Override
+    public UserResponse createUser(UserRequest userRequest) {
+        return createUserInternal(userRequest, null);
+    }
+
+    public UserResponse createUserByLocality(UserRequest userRequest, String localityCode) {
+        return createUserInternal(userRequest, localityCode);
+    }

Committable suggestion skipped: line range outside the PR's diff.

---

319-321: 💡 Codebase verification

Role codes should not be directly mapped to skill types and levels

The current implementation is problematic because:

• No validation exists to ensure role codes are valid skill types/levels
• Using the same role code for both type and level lacks semantic meaning
• No configuration exists to map roles to appropriate skill levels

Recommendations:

• Introduce a mapping configuration to translate role codes to appropriate skill types
• Define separate logic for determining skill levels
• Add validation to ensure only valid skill types and levels are assigned

🔗 Analysis chain

Verify the role-to-skill mapping logic.

The current implementation assumes that role codes can directly map to both skill type and level. This might lead to incorrect skill assignments if role codes don't align with actual skill levels.

Consider:

1. Should there be a mapping configuration for role codes to skill levels?
2. Is it appropriate to use the same value for both type and level?

Let's verify the role codes in use:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for role definitions to understand the available role codes
rg -A 5 "role.*code.*=" 

Length of output: 5152

---

Script:

#!/bin/bash
# Search for skill-related configurations and type/level definitions
echo "=== Searching for skill configurations ==="
rg -g "!*.{log,lock}" "skill.*type|skill.*level" -B 2 -A 2

echo -e "\n=== Searching for role definitions in configuration files ==="
fd -e yaml -e json -e properties | xargs rg "roles:|role:|ROLE_" -B 2 -A 2

echo -e "\n=== Searching for other skill mapping implementations ==="
ast-grep --pattern 'Skill.builder()' -A 5

Length of output: 26400

core-services/egov-hrms/src/main/java/org/egov/hrms/web/contract/EmailRequest.java (1)

12-16: 🛠️ Refactor suggestion

Add validation annotations to ensure request integrity.

Consider adding validation annotations to ensure the request fields are properly validated:

 public class EmailRequest {
 
+    @NotNull(message = "Request info is required")
     private RequestInfo requestInfo;
+    @NotNull(message = "Email details are required")
     private Email email;
 }

📝 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 class EmailRequest {

    @NotNull(message = "Request info is required")
    private RequestInfo requestInfo;
    @NotNull(message = "Email details are required")
    private Email email;
}

core-services/egov-hrms/src/main/java/org/egov/hrms/web/contract/Email.java (1)

11-16: 🧹 Nitpick (assertive)

Remove redundant Lombok annotation.

The @Data annotation is redundant since you're already using @Getter, @Setter, @NoArgsConstructor, and @AllArgsConstructor.

 @Setter
 @Getter
 @NoArgsConstructor
 @AllArgsConstructor
-@Data
 @Builder

📝 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.

@Setter
@Getter
@NoArgsConstructor
@AllArgsConstructor
@Builder

core-services/egov-hrms/src/main/java/org/egov/hrms/consumer/HrmsConsumer.java (1)

35-45: 🛠️ Refactor suggestion

Enhance error handling and logging security.

The current implementation has several areas for improvement:

1. Add request validation:

 @KafkaListener(topics = {"${kafka.topics.hrms.updateData}", "${kafka.topics.hrms.email.notification}"})
 public void listenUpdateEmployeeData(final HashMap<String, Object> record,@Header(KafkaHeaders.RECEIVED_TOPIC) String topic) {
     try {
         EmployeeRequest employeeRequest = mapper.convertValue(record, EmployeeRequest.class);
+        validateRequest(employeeRequest);
 
         if(topic.equals(propertiesManager.getHrmsEmailNotifTopic())) {

2. Improve exception handling and secure logging:

-    } catch (final Exception e) {
-        log.error("Error while listening to value: " + record + " on topic: " + topic + ": ", e);
+    } catch (IllegalArgumentException e) {
+        log.error("Error deserializing message from topic: {}", topic, e);
+    } catch (Exception e) {
+        log.error("Error processing message from topic: {}", topic, e);
     }
 }
+
+private void validateRequest(EmployeeRequest request) {
+    if (request == null || request.getRequestInfo() == null) {
+        throw new IllegalArgumentException("Invalid request format");
+    }
+}

The changes:

• Add input validation to prevent null pointer exceptions
• Separate exception handling for different error types
• Remove sensitive data from error logs
• Add a validation helper method

Committable suggestion skipped: line range outside the PR's diff.

core-services/egov-hrms/src/main/java/org/egov/hrms/config/PropertiesManager.java (2)

90-91: 🧹 Nitpick (assertive)

Improve boolean property naming and add default value.

Follow Java naming conventions for boolean properties and provide a default value:

-@Value("${notification.email.enabled}")
-private Boolean isEmailNotificationEnabled;
+@Value("${notification.email.enabled:false}")
+private boolean emailNotificationEnabled;

📝 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.

	@Value("${notification.email.enabled:false}")
	private boolean emailNotificationEnabled;

---

152-156: 🛠️ Refactor suggestion

Add default values for new properties.

Provide default values to prevent null pointer exceptions if properties are not configured:

-@Value("${hrms.email.notification.implementation.partner}")
+@Value("${hrms.email.notification.implementation.partner:}")
 public String emailNotificationImplementationPartner;

-@Value("${hrms.email.notification.website.link}")
+@Value("${hrms.email.notification.website.link:}")
 public String emailNotificationWebsiteLink;

📝 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.

	@Value("${hrms.email.notification.implementation.partner:}")
	public String emailNotificationImplementationPartner;

	@Value("${hrms.email.notification.website.link:}")
	public String emailNotificationWebsiteLink;

core-services/egov-hrms/src/main/java/org/egov/hrms/service/NotificationService.java (1)

211-227: 🧹 Nitpick (assertive)

Enhance error logging in processEmailNotification.

While the error handling is good, the log messages could be more detailed to aid in debugging.

Apply this diff to improve the logging:

-            log.error("Invalid employee request received for email notification");
+            log.error("Invalid employee request received for email notification. Request: {}", employeeRequest);

-            log.error("Error processing email notification for given employees");
+            log.error("Error processing email notification for employees: {}", e.getMessage(), 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.

	public void processEmailNotification(EmployeeRequest employeeRequest) {
		if (employeeRequest == null || CollectionUtils.isEmpty(employeeRequest.getEmployees())) {
			log.error("Invalid employee request received for email notification. Request: {}", employeeRequest);
			return;
		}
		try {
			// Fetch localization messages and get email message template for HEALTH_HRMS_EMAIL_LOCALIZATION_CODE template code.
			String localizationMessages = notificationUtil.getLocalizationMessages(employeeRequest);
			String messageTemplate = notificationUtil.getMessageTemplate(HEALTH_HRMS_EMAIL_LOCALIZATION_CODE, localizationMessages);

			// Create email requests from the employee details provided in the employeeRequest.
			List<EmailRequest> emailRequests = notificationUtil.createEmailRequest(employeeRequest, messageTemplate);
			notificationUtil.sendEmail(emailRequests);
		} catch (Exception e) {
			log.error("Error processing email notification for employees: {}", e.getMessage(), e);
		}
	}

core-services/egov-hrms/src/main/resources/application.properties (1)

123-127: 🛠️ Refactor suggestion

Remove hardcoded website link from properties file.

According to previous discussions, hrms.email.notification.website.link should be managed through the DevOps chart to support environment-specific values.

Remove the hardcoded value and ensure it's configured in the DevOps chart instead.

health-services/libraries/health-services-models/src/main/java/org/egov/common/models/household/HouseHoldType.java (2)

15-19: 🧹 Nitpick (assertive)

Add toString/JsonValue annotation for consistent serialization

For consistency with BeneficiaryType, add @JsonValue annotation and override toString.

 private String value;

 HouseHoldType(String value) {
     this.value = value;
 }
+
+@Override
+@JsonValue
+public String toString() {
+    return String.valueOf(value);
+}

📝 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 String value;

	HouseHoldType(String value) {
		this.value = value;
	}

	@Override
	@JsonValue
	public String toString() {
		return String.valueOf(value);
	}

---

21-29: ⚠️ Potential issue

Handle invalid enum values more gracefully

The fromValue method returns null for invalid values, which could lead to NullPointerExceptions. Consider throwing an IllegalArgumentException with a descriptive message instead.

 @JsonCreator
 public static HouseHoldType fromValue(String text) {
     for (HouseHoldType b : HouseHoldType.values()) {
         if (String.valueOf(b.value).equalsIgnoreCase(text)) {
             return b;
         }
     }
-    return null;
+    throw new IllegalArgumentException("Unknown HouseHoldType value: " + text);
 }

📝 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.

	@JsonCreator
	public static HouseHoldType fromValue(String text) {
		for (HouseHoldType b : HouseHoldType.values()) {
			if (String.valueOf(b.value).equalsIgnoreCase(text)) {
				return b;
			}
		}
		throw new IllegalArgumentException("Unknown HouseHoldType value: " + text);
	}

health-services/libraries/health-services-models/src/main/java/org/egov/common/models/project/BeneficiaryType.java (1)

25-34: ⚠️ Potential issue

Handle invalid enum values more gracefully

Similar to HouseHoldType, the fromValue method returns null for invalid values. Consider throwing an IllegalArgumentException instead.

 @JsonCreator
 public static BeneficiaryType fromValue(String text) {
     for(BeneficiaryType a:BeneficiaryType.values()){
         if(String.valueOf(a.value).equalsIgnoreCase(text)){
             return a;
         }
     }
-    return null;
+    throw new IllegalArgumentException("Unknown BeneficiaryType value: " + text);
 }

📝 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.

		@JsonCreator
		public static BeneficiaryType fromValue(String text) {
			for(BeneficiaryType a:BeneficiaryType.values()){
				if(String.valueOf(a.value).equalsIgnoreCase(text)){
					return a;
				}
			}
			throw new IllegalArgumentException("Unknown BeneficiaryType value: " + text);
		}

health-services/libraries/health-services-models/src/main/java/org/egov/common/models/project/Target.java (1)

34-34: 🧹 Nitpick (assertive)

Add validation for required beneficiaryType

Consider adding @NotNull validation to ensure beneficiaryType is always provided, as it seems to be a crucial field for target definition.

 @JsonProperty("beneficiaryType")
+@NotNull
 private BeneficiaryType beneficiaryType = null;

📝 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("beneficiaryType")
    @NotNull
    private BeneficiaryType beneficiaryType = null;

health-services/libraries/health-services-models/src/main/java/org/egov/common/models/household/Household.java (1)

37-39: 🧹 Nitpick (assertive)

Add validation for householdType field

The new householdType field lacks validation. Consider adding @NotNull if this is a required field.

 @JsonProperty("householdType")
+@NotNull
 private HouseHoldType householdType = null;

📝 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("householdType")
    @NotNull
    private HouseHoldType householdType = null;

health-services/libraries/health-services-models/src/main/java/org/egov/common/models/household/HouseholdSearch.java (1)

40-41: 🛠️ Refactor suggestion

Consider using HouseHoldType enum instead of String.

The householdType field should be of type HouseHoldType enum instead of String to maintain type safety and consistency with other related changes in the codebase.

    @JsonProperty("householdType")
-    private String householdType = null;
+    private HouseHoldType householdType = null;

Committable suggestion skipped: line range outside the PR's diff.

health-services/household/src/main/java/org/egov/household/config/HouseholdConfiguration.java (2)

49-50: 🧹 Nitpick (assertive)

Add default value for communityHouseholdCreatorRoleCode.

Consider providing a default value in the annotation to ensure graceful behavior if the configuration is missing.

-    @Value("${household.type.community.creator.role}")
+    @Value("${household.type.community.creator.role:COMMUNITY_HOUSEHOLD_CREATOR}")
    private String communityHouseholdCreatorRoleCode;

📝 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.

    @Value("${household.type.community.creator.role:COMMUNITY_HOUSEHOLD_CREATOR}")
    private String communityHouseholdCreatorRoleCode;

---

46-47: 🧹 Nitpick (assertive)

Add default value for householdTypeSameValidation.

Consider providing a default value in the annotation to ensure graceful behavior if the configuration is missing.

-    @Value("${household.type.same.validation}")
+    @Value("${household.type.same.validation:false}")
    private boolean householdTypeSameValidation;

📝 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.

    @Value("${household.type.same.validation:false}")
    private boolean householdTypeSameValidation;

health-services/libraries/health-services-models/src/main/java/org/egov/common/models/project/ProjectType.java (1)

51-51: 🧹 Nitpick (assertive)

Add validation annotations for beneficiaryType.

Consider adding validation annotations to ensure the field is properly validated during object creation and updates.

+    @Valid
+    @NotNull
    private BeneficiaryType beneficiaryType = null;

📝 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.

    @Valid
    @NotNull
    private BeneficiaryType beneficiaryType = null;

health-services/household/src/main/java/org/egov/household/repository/rowmapper/HouseholdRowMapper.java (1)

40-40: ⚠️ Potential issue

Handle potential null householdType values.

The direct call to fromValue() on the result of getString("householdType") could throw a NullPointerException if the householdType column contains NULL values in the database.

Apply this diff to safely handle null values:

-                    .householdType(HouseHoldType.fromValue(resultSet.getString("householdType")))
+                    .householdType(resultSet.getString("householdType") != null ? 
+                        HouseHoldType.fromValue(resultSet.getString("householdType")) : null)

📝 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.

                    .householdType(resultSet.getString("householdType") != null ? 
                        HouseHoldType.fromValue(resultSet.getString("householdType")) : null)

health-services/household/src/main/java/org/egov/household/repository/HouseholdRepository.java (1)

85-88: 🛠️ Refactor suggestion

Refactor duplicated logic and avoid hardcoded strings.

The logic for handling FAMILY type is duplicated between find and findByRadius methods. Additionally, the "COMMUNITY" string is hardcoded.

1. Extract the common logic to a private method:

private String getHouseholdTypeCondition(String originalCondition, HouseholdSearch searchObject) {
    if (searchObject.getHouseholdType() != null && 
        searchObject.getHouseholdType().equalsIgnoreCase(HouseHoldType.FAMILY.name())) {
        return originalCondition.replace(
            "householdType=:householdType", 
            String.format("(householdType!='%s' OR householdType IS NULL)", 
                HouseHoldType.COMMUNITY.name())
        );
    }
    return originalCondition;
}

2. Use the extracted method in both places:

-        if (searchObject.getHouseholdType() != null && searchObject.getHouseholdType().equalsIgnoreCase("FAMILY")) {
-            query = query.replace("householdType=:householdType", "(householdType!='COMMUNITY' OR householdType IS NULL)");
-        }
+        query = getHouseholdTypeCondition(query, searchObject);

Also applies to: 136-139

health-services/household/src/main/java/org/egov/household/service/HouseholdService.java (1)

58-60: 🧹 Nitpick (assertive)

Document the purpose and ordering of validators.

The new validators have been added without documentation explaining their purpose or why they need to run in this specific order. This makes it difficult for other developers to understand the validation flow.

Add comments explaining each validator's purpose and any ordering dependencies:

     private final Predicate<Validator<HouseholdBulkRequest, Household>> isApplicableForCreate = validator ->
             validator.getClass().equals(HBoundaryValidator.class)
+            // Validates if the entity already exists
             || validator.getClass().equals(HExistentEntityValidator.class)
+            // Validates community-specific rules
             || validator.getClass().equals(HCommunityValidator.class)
+            // Validates household type constraints
             || validator.getClass().equals(HCommunityTypeValidator.class);

     private final Predicate<Validator<HouseholdBulkRequest, Household>> isApplicableForUpdate = validator ->
             validator.getClass().equals(HNullIdValidator.class)
             || validator.getClass().equals(HBoundaryValidator.class)
             || validator.getClass().equals(HIsDeletedValidator.class)
             || validator.getClass().equals(HUniqueEntityValidator.class)
             || validator.getClass().equals(HNonExistentEntityValidator.class)
             || validator.getClass().equals(HRowVersionValidator.class)
+            // Validates community-specific rules
             || validator.getClass().equals(HCommunityValidator.class)
+            // Validates household type constraints
             || validator.getClass().equals(HCommunityTypeValidator.class)
+            // Validates changes to household type
             || validator.getClass().equals(HHouseholdTypeChangeValidator.class);

Also applies to: 68-71

health-services/project/src/main/java/org/egov/project/validator/beneficiary/BeneficiaryValidator.java (1)

116-127: 🧹 Nitpick (assertive)

Modernize switch statement and improve error message.

The switch statement could be modernized using the enhanced switch syntax, and the error message could be more descriptive.

Apply this diff to modernize the code:

-    private void searchBeneficiary(BeneficiaryType beneficiaryType, List<ProjectBeneficiary> beneficiaryList,
-                                   RequestInfo requestInfo, String tenantId,
-                                   Map<ProjectBeneficiary, List<Error>> errorDetailsMap) {
-        switch (beneficiaryType) {
-            case HOUSEHOLD:
-                searchHouseholdBeneficiary(beneficiaryList, requestInfo, tenantId, errorDetailsMap);
-                break;
-            case INDIVIDUAL:
-                searchIndividualBeneficiary(beneficiaryList, requestInfo, tenantId, errorDetailsMap);
-                break;
-            default:
-                throw new CustomException("INVALID_BENEFICIARY_TYPE", beneficiaryType.name());
-        }
-    }
+    private void searchBeneficiary(BeneficiaryType beneficiaryType, List<ProjectBeneficiary> beneficiaryList,
+                                   RequestInfo requestInfo, String tenantId,
+                                   Map<ProjectBeneficiary, List<Error>> errorDetailsMap) {
+        switch (beneficiaryType) {
+            case HOUSEHOLD -> searchHouseholdBeneficiary(beneficiaryList, requestInfo, tenantId, errorDetailsMap);
+            case INDIVIDUAL -> searchIndividualBeneficiary(beneficiaryList, requestInfo, tenantId, errorDetailsMap);
+            default -> throw new CustomException("INVALID_BENEFICIARY_TYPE",
+                String.format("Beneficiary type '%s' is not supported. Supported types are: %s",
+                    beneficiaryType.name(),
+                    java.util.Arrays.toString(BeneficiaryType.values())));
+        }
+    }

📝 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 void searchBeneficiary(BeneficiaryType beneficiaryType, List<ProjectBeneficiary> beneficiaryList,
                                   RequestInfo requestInfo, String tenantId,
                                   Map<ProjectBeneficiary, List<Error>> errorDetailsMap) {
        switch (beneficiaryType) {
            case HOUSEHOLD -> searchHouseholdBeneficiary(beneficiaryList, requestInfo, tenantId, errorDetailsMap);
            case INDIVIDUAL -> searchIndividualBeneficiary(beneficiaryList, requestInfo, tenantId, errorDetailsMap);
            default -> throw new CustomException("INVALID_BENEFICIARY_TYPE",
                String.format("Beneficiary type '%s' is not supported. Supported types are: %s",
                    beneficiaryType.name(),
                    java.util.Arrays.toString(BeneficiaryType.values())));
        }
    }

health-services/household/src/main/resources/application.properties (1)

107-109: 🧹 Nitpick (assertive)

Document the new configuration properties.

The new household type configuration properties would benefit from inline documentation explaining their purpose and possible values.

 #Community Household Type
+# Controls whether to validate if households of the same type exist in the locality
 household.type.same.validation=true
+# Role required to create community type households
 household.type.community.creator.role=COMMUNITY_CREATOR

📝 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.

#Community Household Type
# Controls whether to validate if households of the same type exist in the locality
household.type.same.validation=true
# Role required to create community type households
household.type.community.creator.role=COMMUNITY_CREATOR

health-services/household/src/main/resources/household-persister.yml (1)

9-9: 💡 Codebase verification

Invalid INSERT query: 'householdType' column not found in table schema

The INSERT query includes a 'householdType' column that does not exist in the HOUSEHOLD table schema. This will cause the query to fail.

• Remove 'householdType' from the INSERT query in household-persister.yml or add the column to the table schema if it's required.

🔗 Analysis chain

Verify the column order in INSERT query.

The column order in the INSERT query should match the order of values and JSON mappings.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check the table schema to verify column order
echo "SELECT column_name, ordinal_position 
FROM information_schema.columns 
WHERE table_name = 'household' 
ORDER BY ordinal_position;" > verify_columns.sql

# Note: This is just to verify the schema, actual execution would be done in the database
cat verify_columns.sql

Length of output: 299

---

Script:

#!/bin/bash
# Search for CREATE TABLE household in SQL files
rg -i "create table.*household" -A 20

Length of output: 7403