HCMPRE-492 Changes for plan estimation workflow integration #978
Conversation
Priyanka-eGov
requested review from
kavi-egov and
sathishp-eGov
as code owners
|
@CodeRabbit review |
✅ Actions performedReview triggered.
|
WalkthroughThis pull request introduces several modifications across multiple classes in the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant PlanService
participant WorkflowService
participant PlanValidator
participant CommonUtil
User->>PlanService: createPlan(PlanRequest)
PlanService->>WorkflowService: invokeWorkflowForStatusUpdate(PlanRequest)
WorkflowService->>CommonUtil: validateUserInfo(RequestInfo)
PlanService->>PlanValidator: validatePlanCreate(PlanRequest)
PlanValidator->>PlanEmployeeService: validatePlanEmployeeAssignmentAndJurisdiction(PlanRequest)
PlanService-->>User: Plan created successfully
Possibly related PRs
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 16
🧹 Outside diff range comments (2)
health-services/plan-service/src/main/java/digit/service/validator/PlanEmployeeAssignmentValidator.java (1)
Consistent Role Handling Needed in
PlanEmployeeAssignmentClassThe current implementation of
getRole()returns a single role, while the recent change introduces a list-based approach usingCollections.singletonList(). This inconsistency can lead to potential issues in role validation and processing.
- Update the
PlanEmployeeAssignmentclass to handle roles consistently as either single roles or lists across all methods.- Refactor related methods to align with the chosen approach for role handling.
🔗 Analysis chain
Line range hint
132-149: Consider reviewing thePlanEmployeeAssignmentclass for consistencyWhile the change to use
Collections.singletonList()is appropriate, it highlights a potential inconsistency in how roles are handled throughout the codebase. TheplanEmployeeAssignment.getRole()method seems to return a single role, which may not be consistent with the list-based approach introduced by this change.To ensure consistency across the codebase, please review the
PlanEmployeeAssignmentclass and related classes. Consider updating them to use a list of roles instead of a single role if appropriate. You can use the following script to locate relevant files:This will help identify areas where the role handling might need to be updated for consistency with the new list-based approach.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for PlanEmployeeAssignment class definition and usages echo "Searching for PlanEmployeeAssignment class definition:" rg --type java "class PlanEmployeeAssignment" echo "\nSearching for getRole() method in PlanEmployeeAssignment:" rg --type java "getRole\(\)" -C 5 echo "\nSearching for usages of getRole() method:" rg --type java "\.getRole\(\)" -C 2Length of output: 18591
health-services/plan-service/src/main/java/digit/service/validator/PlanConfigurationValidator.java (1)
Line range hint
1-638: Overall assessment: Good refactoring for improved maintainability.The changes made to this file are consistent and improve code maintainability by centralizing the user info validation logic. The addition of the
RequestInfoimport and the use ofcommonUtil.validateUserInfo()in bothvalidateCreateandvalidateUpdateRequestmethods are positive changes.To ensure the refactoring hasn't introduced any regressions:
- Verify the implementation of
CommonUtil.validateUserInfo()as suggested earlier.- Run the existing test suite to confirm that the validation behavior remains unchanged.
- Consider adding or updating tests to specifically cover the user info validation scenarios.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
📒 Files selected for processing (16)
- health-services/plan-service/src/main/java/digit/config/Configuration.java (2 hunks)
- health-services/plan-service/src/main/java/digit/config/ServiceConstants.java (3 hunks)
- health-services/plan-service/src/main/java/digit/repository/querybuilder/PlanEmployeeAssignmentQueryBuilder.java (2 hunks)
- health-services/plan-service/src/main/java/digit/repository/querybuilder/PlanQueryBuilder.java (3 hunks)
- health-services/plan-service/src/main/java/digit/repository/rowmapper/PlanRowMapper.java (1 hunks)
- health-services/plan-service/src/main/java/digit/service/PlanService.java (3 hunks)
- health-services/plan-service/src/main/java/digit/service/PlanValidator.java (5 hunks)
- health-services/plan-service/src/main/java/digit/service/validator/PlanConfigurationValidator.java (3 hunks)
- health-services/plan-service/src/main/java/digit/service/validator/PlanEmployeeAssignmentValidator.java (1 hunks)
- health-services/plan-service/src/main/java/digit/service/workflow/WorkflowService.java (5 hunks)
- health-services/plan-service/src/main/java/digit/util/CommonUtil.java (2 hunks)
- health-services/plan-service/src/main/java/digit/web/models/Plan.java (3 hunks)
- health-services/plan-service/src/main/java/digit/web/models/PlanEmployeeAssignmentSearchCriteria.java (1 hunks)
- health-services/plan-service/src/main/java/digit/web/models/PlanSearchCriteria.java (2 hunks)
- health-services/plan-service/src/main/resources/application.properties (1 hunks)
- health-services/plan-service/src/main/resources/db/migration/main/V20240910200000__plan_add_boundaryAncestralPath_ddl.sql (1 hunks)
🧰 Additional context used
📓 Learnings (5)
health-services/plan-service/src/main/java/digit/config/ServiceConstants.java (2)
Learnt from: Saloni-eGov PR: egovernments/health-campaign-services#906 File: health-services/plan-service/src/main/java/digit/config/ServiceConstants.java:189-193 Timestamp: 2024-09-26T11:17:36.420Z Learning: When reviewing error messages in `ServiceConstants.java`, avoid suggesting changes to make them more descriptive unless specifically requested.Learnt from: Saloni-eGov PR: egovernments/health-campaign-services#906 File: health-services/plan-service/src/main/java/digit/config/ServiceConstants.java:189-193 Timestamp: 2024-10-08T20:11:07.772Z Learning: When reviewing error messages in `ServiceConstants.java`, avoid suggesting changes to make them more descriptive unless specifically requested.health-services/plan-service/src/main/java/digit/service/PlanValidator.java (2)
Learnt from: Priyanka-eGov PR: egovernments/health-campaign-services#902 File: health-services/plan-service/src/main/java/digit/service/validator/PlanConfigurationValidator.java:553-553 Timestamp: 2024-09-25T08:41:31.377Z Learning: The plan-service project is using Java 17.Learnt from: Priyanka-eGov PR: egovernments/health-campaign-services#902 File: health-services/plan-service/src/main/java/digit/service/validator/PlanConfigurationValidator.java:553-553 Timestamp: 2024-10-08T20:11:07.772Z Learning: The plan-service project is using Java 17.health-services/plan-service/src/main/java/digit/service/validator/PlanConfigurationValidator.java (2)
Learnt from: Priyanka-eGov PR: egovernments/health-campaign-services#902 File: health-services/plan-service/src/main/java/digit/service/validator/PlanConfigurationValidator.java:553-553 Timestamp: 2024-09-25T08:41:31.377Z Learning: The plan-service project is using Java 17.Learnt from: Priyanka-eGov PR: egovernments/health-campaign-services#902 File: health-services/plan-service/src/main/java/digit/service/validator/PlanConfigurationValidator.java:553-553 Timestamp: 2024-10-08T20:11:07.772Z Learning: The plan-service project is using Java 17.health-services/plan-service/src/main/java/digit/web/models/PlanSearchCriteria.java (1)
Learnt from: Priyanka-eGov PR: egovernments/health-campaign-services#737 File: health-services/plan-service/src/main/java/digit/web/models/PlanConfigurationSearchCriteria.java:34-35 Timestamp: 2024-10-08T20:11:07.773Z Learning: In the `PlanConfigurationSearchCriteria` class, the `status` field can be empty, and no validation constraints are required for this field.health-services/plan-service/src/main/resources/db/migration/main/V20240910200000__plan_add_boundaryAncestralPath_ddl.sql (2)
Learnt from: Taniya-eGov PR: egovernments/health-campaign-services#914 File: health-services/plan-service/src/main/resources/db/migration/main/V20240923113045__plan_facility_create_ddl.sql:8-8 Timestamp: 2024-09-27T06:09:27.293Z Learning: In the `plan_facility_linkage` table, the `service_boundaries` column is intentionally defined as `varchar(2048)` and does not need to be changed to an array or JSONB.Learnt from: Taniya-eGov PR: egovernments/health-campaign-services#914 File: health-services/plan-service/src/main/resources/db/migration/main/V20240923113045__plan_facility_create_ddl.sql:8-8 Timestamp: 2024-10-08T20:11:07.772Z Learning: In the `plan_facility_linkage` table, the `service_boundaries` column is intentionally defined as `varchar(2048)` and does not need to be changed to an array or JSONB.
🔇 Additional comments (31)
health-services/plan-service/src/main/resources/db/migration/main/V20240910200000__plan_add_boundaryAncestralPath_ddl.sql (1)
1-4: 🧹 Nitpick (assertive)Approve changes with suggestions for improvement
The alterations to the
plantable align with the PR objectives and enhance the data model. However, there are a few points to consider:
For
boundary_ancestral_path:
- The use of TEXT is suitable for storing hierarchical data, but consider implementing a maximum length constraint if possible to prevent excessive data storage.
- Since it's NOT NULL, ensure that all existing rows in the
plantable can be populated with a valid value during migration.For
status:
- Consider adding a CHECK constraint to limit the possible values for the status field, ensuring data integrity.
For
assignee:
- The nullable design is appropriate, allowing plans to be unassigned.
General suggestion:
- For the NOT NULL columns (
boundary_ancestral_pathandstatus), consider adding DEFAULT values to ease future insertions and prevent issues with existing data.Here's a suggested refinement:
ALTER TABLE plan ADD boundary_ancestral_path TEXT NOT NULL DEFAULT '', ADD status character varying(64) NOT NULL DEFAULT 'draft', ADD assignee varchar(64), ADD CONSTRAINT check_status CHECK (status IN ('draft', 'active', 'completed', 'cancelled'));This suggestion adds default values and a CHECK constraint for the status field. Adjust the status values in the CHECK constraint according to your specific requirements.
To ensure these changes don't conflict with existing data or constraints, please run the following verification script:
Please review the script output to ensure the changes are compatible with the existing data and schema.
health-services/plan-service/src/main/java/digit/web/models/PlanEmployeeAssignmentSearchCriteria.java (1)
41-41: Approve the change ofrolefield type toList<String>The modification of the
rolefield fromStringtoList<String>is consistent with the AI-generated summary and appears to be a valid change. This update allows for multiple roles to be assigned, which can provide more flexibility in role management.However, there are a few considerations:
- Ensure that all code that uses this class is updated to handle a list of roles instead of a single role.
- Update any documentation or API specifications to reflect this change.
- Consider adding a null check and empty list check when processing this field to avoid potential
NullPointerExceptions.To ensure this change is consistently applied throughout the codebase, please run the following script:
This script will help identify areas of the codebase that might need updates due to this change.
✅ Verification successful
To ensure all usages of the
rolefield inPlanEmployeeAssignmentSearchCriteriaare correctly updated to handle a list of roles, please run the following scripts:
Verified the change of
rolefield type toList<String>.The
rolefield inPlanEmployeeAssignmentSearchCriteria.javahas been successfully updated toList<String>. No additional usages or manipulations of this field were found within the class, indicating that the change is safe and consistent across the codebase.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for usages of PlanEmployeeAssignmentSearchCriteria and its role field # Search for PlanEmployeeAssignmentSearchCriteria usage echo "Searching for PlanEmployeeAssignmentSearchCriteria usage:" rg --type java "PlanEmployeeAssignmentSearchCriteria" -C 3 # Search for specific usage of the role field echo "\nSearching for specific usage of the role field:" rg --type java "PlanEmployeeAssignmentSearchCriteria.*role" -C 3 # Search for potential places where single role might still be used echo "\nSearching for potential places where single role might still be used:" rg --type java "PlanEmployeeAssignmentSearchCriteria.*getRole\\(\\)" -C 3Length of output: 17223
Script:
#!/bin/bash # Description: Comprehensive search for all interactions with the 'role' field in PlanEmployeeAssignmentSearchCriteria # Search for any assignments to the role field echo "Searching for assignments to the role field:" rg --type java "PlanEmployeeAssignmentSearchCriteria.*role\s*=" -C 3 # Search for any method calls that involve the role field, such as setters or custom methods echo "\nSearching for method interactions with the role field:" rg --type java "PlanEmployeeAssignmentSearchCriteria.*setRole|addRole|removeRole|updateRole" -C 3 # Search for any iterations or stream operations on the role field echo "\nSearching for iterations or stream operations on the role field:" rg --type java "PlanEmployeeAssignmentSearchCriteria.*role\.stream\(" -C 3Length of output: 7209
health-services/plan-service/src/main/java/digit/web/models/PlanSearchCriteria.java (4)
3-3: LGTM: Import statement for List is appropriate.The addition of
import java.util.List;is necessary and correct for the newjurisdictionfield.
42-43: LGTM: Status field added correctly.The
statusfield is appropriately added with the correct type and annotation. The absence of validation constraints aligns with the previous learning that the status field can be empty.
Line range hint
1-58: Overall, the changes to PlanSearchCriteria.java look good and align with the PR objectives.The additions of
status,assignee, andjurisdictionfields enhance the search criteria capabilities as intended. The implementation is clean and follows good practices. Consider the suggestions for potential validation improvements, particularly for theassigneeandjurisdictionfields.
45-46: LGTM: Assignee field added correctly. Consider validation requirements.The
assigneefield is appropriately added with the correct type and annotation. However, consider whether any validation constraints (e.g.,@NotNullor@Size) should be applied to this field.To verify the usage and potential validation requirements for the
assigneefield, you can run the following script:health-services/plan-service/src/main/java/digit/web/models/Plan.java (2)
10-10: LGTM: Workflow import added correctlyThe import for the
Workflowclass fromorg.egov.common.contract.modelsis correctly placed and aligns with the newworkflowproperty in thePlanclass.
75-77: LGTM: workflow property added correctlyThe
workflowproperty has been correctly added to thePlanclass with appropriate@JsonPropertyand@Validannotations. The@Validannotation ensures that theWorkflowobject itself will be validated, which is excellent for maintaining data integrity with nested objects.The property type
Workflowcorresponds to the newly added import, ensuring consistency in the class structure.health-services/plan-service/src/main/java/digit/config/Configuration.java (2)
9-10: LGTM: Import statement added correctly.The import for
java.util.Listhas been added appropriately to support the new fieldplanEstimationApproverRoles.
25-26: New configuration field for plan estimation approver roles.The addition of
planEstimationApproverRolesaligns with the PR objectives for enhancing the plan estimation workflow. This field will allow the application to configure and access the list of roles that can approve plan estimations.A few points to consider:
- Ensure that the corresponding property
plan.estimation.approver.rolesis defined in your application properties file.- Document the expected format of this property (e.g., comma-separated list of role names).
- Consider adding a brief comment explaining the purpose of this field for future maintainers.
To ensure the property is correctly defined, let's check the application properties:
✅ Verification successful
Verified:
plan.estimation.approver.rolesis defined inapplication.properties.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the presence of plan.estimation.approver.roles in application properties # Test: Search for the property in application.properties or application.yml files rg --type yaml --type properties 'plan\.estimation\.approver\.roles' src/main/resources/Length of output: 125
Script:
#!/bin/bash # Description: Verify the presence of plan.estimation.approver.roles in application properties # Test: Search for the property in application.properties or application.yml files rg 'plan\.estimation\.approver\.roles' src/main/resources/application.{properties,yml}Length of output: 274
Script:
#!/bin/bash # Description: Recursively search for plan.estimation.approver.roles in the entire codebase rg 'plan\.estimation\.approver\.roles' .Length of output: 329
health-services/plan-service/src/main/java/digit/service/PlanService.java (4)
4-4: LGTM: Import statement added correctly.The import for
WorkflowServiceis appropriately placed and aligns with the new dependency introduced in the class.
25-26: LGTM: WorkflowService field added correctly.The
workflowServicefield is appropriately declared as private and follows the established naming convention in the class.
27-31: LGTM: Constructor updated correctly to include WorkflowService.The constructor has been properly modified to include the new
WorkflowServiceparameter. The new dependency is correctly assigned to the corresponding field while maintaining existing parameters and assignments.
Line range hint
1-95: Overall, the changes look good with a minor suggestion for improvement.The integration of
WorkflowServiceinto thePlanServiceclass has been implemented correctly. The new dependency is properly injected, and thecreatePlanmethod has been updated to include the workflow status update. The code structure and style remain consistent with the existing implementation.The only suggestion for improvement is to add error handling for the workflow invocation in the
createPlanmethod, as mentioned in the previous comment. This will enhance the robustness of the service.health-services/plan-service/src/main/java/digit/repository/querybuilder/PlanEmployeeAssignmentQueryBuilder.java (1)
9-9: LGTM: Import statement addition is appropriate.The addition of
java.util.LinkedHashSetimport is consistent with the changes in the query building logic. LinkedHashSet is a good choice as it maintains insertion order, which can be important for consistent query generation.health-services/plan-service/src/main/java/digit/util/CommonUtil.java (2)
9-12: LGTM: New imports are correctly added and necessary.The new imports for
RequestInfoandObjectUtilsare correctly placed and are required for the newvalidateUserInfomethod. They follow the existing import structure of the file.
148-160: LGTM: NewvalidateUserInfomethod is well-implemented.The new method is correctly implemented and serves a clear purpose. It follows existing coding conventions and includes proper error handling with both logging and exception throwing.
A minor note:
- Ensure that the constants
USERINFO_MISSING_CODEandUSERINFO_MISSING_MESSAGEare defined in the appropriate constants file (likelyServiceConstants.java).To verify the constants, please run the following script:
✅ Verification successful
LGTM:
validateUserInfomethod and related constants are properly implemented.The new method is correctly implemented and serves a clear purpose. It follows existing coding conventions and includes proper error handling with both logging and exception throwing.
The constants
USERINFO_MISSING_CODEandUSERINFO_MISSING_MESSAGEare properly defined inServiceConstants.java.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that USERINFO_MISSING_CODE and USERINFO_MISSING_MESSAGE constants are defined. # Test: Search for the constants in the ServiceConstants file rg --type java 'USERINFO_MISSING_(CODE|MESSAGE)' health-services/plan-service/src/main/java/digit/config/ServiceConstants.javaLength of output: 301
health-services/plan-service/src/main/java/digit/repository/rowmapper/PlanRowMapper.java (1)
57-57: LGTM: New field added correctly.The addition of the
boundaryAncestralPathfield to thePlanobject is implemented correctly and follows the existing pattern for mapping database columns to object properties.To ensure consistency across the codebase, please run the following script to check for usage of the new
boundaryAncestralPathfield:This script will help identify if the new field is consistently used across the codebase and if there are any places where it might have been missed.
✅ Verification successful
Verified:
boundaryAncestralPathis consistently used across the codebase.The
boundaryAncestralPathfield has been correctly added to thePlanobject and is consistently referenced in both Java and SQL files. No missing usages or inconsistencies were found.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for consistent usage of the new boundaryAncestralPath field # Search for references to boundaryAncestralPath in Java files echo "Searching for references to boundaryAncestralPath:" rg --type java 'boundaryAncestralPath' # Search for references to plan_boundary_ancestral_path in SQL files echo "Searching for references to plan_boundary_ancestral_path in SQL files:" rg --type sql 'plan_boundary_ancestral_path' # Search for potential missed usages of the new field echo "Checking for potential missed usages:" rg --type java 'class Plan' -A 20 | grep -v 'boundaryAncestralPath'Length of output: 120034
health-services/plan-service/src/main/java/digit/config/ServiceConstants.java (3)
100-101: LGTM: New constants for plan-employee assignment not found error.The new constants
PLAN_EMPLOYEE_ASSIGNMENT_NOT_FOUND_CODEandPLAN_EMPLOYEE_ASSIGNMENT_NOT_FOUND_MESSAGEare well-defined and follow the existing naming conventions. They provide a clear error message for cases where a plan-employee assignment is not found for a given boundary.
258-258: LGTM: New constant for boundary ancestral path JSON field.The new constant
JSON_FIELD_BOUNDARY_ANCESTRAL_PATHis well-defined and follows the existing naming conventions for JSON field constants. It will be useful for handling hierarchical boundary data in JSON format.
272-273: LGTM: New constant for plan estimation business service.The new constant
PLAN_ESTIMATION_BUSINESS_SERVICEis well-defined and follows the existing naming conventions for business service constants. It will be useful for workflow integration related to plan estimation. The added empty line after the constant improves readability.health-services/plan-service/src/main/java/digit/service/validator/PlanConfigurationValidator.java (3)
19-19: LGTM: Import statement added for RequestInfo.The addition of the import statement for
RequestInfois appropriate, as it's likely being used in the updated user info validation process.
481-481: LGTM: Consistent use of CommonUtil for user info validation.This change is consistent with the modification made in the
validateCreatemethod. It reinforces the centralization of user info validation logic in theCommonUtilclass.Please refer to the previous comment regarding the verification of the
CommonUtil.validateUserInfo()method.
93-93: LGTM: User info validation moved to CommonUtil.The change to use
commonUtil.validateUserInfo()instead of a local method is a good refactoring that promotes code reuse. This change simplifies thePlanConfigurationValidatorclass and centralizes the user info validation logic.Please ensure that the
CommonUtilclass'svalidateUserInfomethod provides at least the same level of validation as the original method. You can verify this by running the following command:health-services/plan-service/src/main/java/digit/service/workflow/WorkflowService.java (3)
36-41: Constructor updated to includeCommonUtildependency.The constructor now includes
CommonUtilas a parameter and assigns it to the class membercommonUtil. This is appropriate for the added functionality that relies onCommonUtil.
66-75: MethodinvokeWorkflowForStatusUpdatecorrectly integrates workflow forPlanRequest.The method appropriately handles the workflow integration for
PlanRequestobjects, checking for empty workflows and updating the plan status based on the workflow response.
128-146: MethodcreateWorkflowRequestconstructsProcessInstanceRequestforPlanRequestsuccessfully.The method correctly builds a
ProcessInstanceusing details from thePlanand returns aProcessInstanceRequest. The use of the builder pattern enhances readability and maintainability.health-services/plan-service/src/main/java/digit/repository/querybuilder/PlanQueryBuilder.java (2)
110-114: Verify the Data Type ofassigneeEnsure that the
assigneefield inplanSearchCriteria.getAssignee()matches the data type expected by the database schema. Ifassigneeis intended to be a list or an array of IDs, the current implementation may need to handle multiple values.Would you like assistance in verifying the data types and updating the query accordingly?
116-124:⚠️ Potential issueCorrect the SQL Syntax for Jurisdiction Filtering
The SQL syntax used for filtering by
jurisdictionmay lead to errors. Specifically, the way you're constructing the array and using the&&operator might not yield the intended results. The&&operator checks for overlap between two arrays in PostgreSQL.Apply this diff to correct the query construction:
- builder.append(" ARRAY [ ") - .append(queryUtil.createQuery(planSearchCriteria.getJurisdiction().size())) // Create placeholders (?, ?, ?...) - .append(" ]::text[] "); - - builder.append(" && string_to_array(boundary_ancestral_path, '|') "); + builder.append(" string_to_array(boundary_ancestral_path, '|') && ARRAY[ ") + .append(queryUtil.createQuery(planSearchCriteria.getJurisdiction().size())) // Create placeholders (?, ?, ?...) + .append(" ]::text[] ");This rearranges the query to compare the
boundary_ancestral_patharray with thejurisdictionarray correctly.Here's a script to verify the SQL syntax correctness:
This script checks if the array comparison works as expected.
health-services/plan-service/src/main/java/digit/service/PlanValidator.java (2)
88-89: Confirm the intentional commenting out ofvalidateCampaignId(campaignResponse);The call to
validateCampaignId(campaignResponse);has been commented out in thevalidatePlanCreatemethod. Please verify if this is intentional. Skipping this validation might allow invalid campaign IDs to be processed.
479-480: Verify correct retrieval of plan estimation approver rolesWhen building
PlanEmployeeAssignmentSearchCriteria, the roles are retrieved usingconfig.getPlanEstimationApproverRoles(). Please ensure that this method returns the expected list of roles and that it aligns with the roles used in theplanEmployeeServicesearch logic.If necessary, run the following script to verify the roles:
...rvices/plan-service/src/main/java/digit/web/models/PlanEmployeeAssignmentSearchCriteria.java
Outdated
Show resolved
Hide resolved
health-services/plan-service/src/main/java/digit/web/models/PlanSearchCriteria.java
Show resolved
Hide resolved
health-services/plan-service/src/main/java/digit/web/models/Plan.java
Outdated
Show resolved
Hide resolved
health-services/plan-service/src/main/java/digit/service/PlanService.java
Show resolved
Hide resolved
health-services/plan-service/src/main/java/digit/service/workflow/WorkflowService.java
Show resolved
Hide resolved
health-services/plan-service/src/main/java/digit/service/workflow/WorkflowService.java
Outdated
Show resolved
Hide resolved
health-services/plan-service/src/main/java/digit/repository/querybuilder/PlanQueryBuilder.java
Show resolved
Hide resolved
health-services/plan-service/src/main/java/digit/service/PlanValidator.java
Show resolved
Hide resolved
health-services/plan-service/src/main/java/digit/service/PlanValidator.java
Show resolved
Hide resolved
Ticket link - https://digit-discuss.atlassian.net/browse/HCMPRE-492
Summary by CodeRabbit
Release Notes
New Features
status,assignee,boundaryAncestralPath, andworkflow.status,assignee, andjurisdiction.Improvements
Configuration Changes
plan.estimation.approver.rolesadded to application properties.Database Updates
plantable:boundary_ancestral_path,status, andassignee.