HCMPRE-1922 - Microplanning v0.2 master merge #1340
Conversation
HCMPRE-1661: Adding additional field table in Plan
…health-campaign-services into download-estimation-1578
Enriching additionalField ID after empty check
HCMPRE:1678 - Plan-facility mapping residing boundary validation update
…health-campaign-services into download-estimation-1578
…ownload-estimation-1578
HCMPRE:1782 - Plan-facility integration to fetch fixedPost details
HCMPRE:1817 - Bug Fix - validating plan employee assignments only for active update calls
HCMPRE:1729 - Change the mdms master to adminSchemas in resource generator
HCMPRE-1846 making output columns and facility name cell editable.
HCMPRE:1767 - Resource generator - add Village accessibility details from census into plans
HCMPRE:1768 - Add Facility Name in additional details and enable search on facility name in Plan
Priyanka-eGov
requested review from
kavi-egov and
sathishp-eGov
as code owners
WalkthroughThis pull request introduces enhancements to the plan service, focusing on adding support for additional fields, facility-based searching, and improving data management across multiple components. The changes span various classes in the health services plan service, introducing new fields, methods, and utilities to handle more complex plan-related data structures and search capabilities. Changes
Sequence DiagramsequenceDiagram
participant Client
participant PlanService
participant PlanEnricher
participant PlanRepository
participant Database
Client->>PlanService: Search Plan Request
PlanService->>PlanEnricher: Enrich Search Request
PlanEnricher-->>PlanService: Enriched Request
PlanService->>PlanRepository: Search with Enriched Criteria
PlanRepository->>Database: Query with Additional Filters
Database-->>PlanRepository: Retrieve Matching Plans
PlanRepository-->>PlanService: Plan Results
PlanService-->>Client: Plan Response
Possibly related PRs
Suggested reviewers
Poem
Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
|
@CodeRabbit review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 37
🔭 Outside diff range comments (3)
health-services/plan-service/src/main/java/digit/service/validator/PlanFacilityValidator.java (1)
Line range hint
156-169: Add defensive programming checks.Consider adding the following improvements:
- Add null check for
hierarchyTypeparameter- Handle case where no boundaries match the hierarchy type
- Document the case-insensitive comparison behavior
private Set<String> fetchBoundaryCodes(CampaignDetail campaignDetail, String hierarchyType) { + if (hierarchyType == null) { + throw new CustomException("INVALID_HIERARCHY_TYPE", "Hierarchy type cannot be null"); + } + Set<String> boundaryCodes = campaignDetail.getBoundaries().stream() .filter(boundary -> hierarchyType.equals(boundary.getType().toLowerCase())) .map(Boundary::getCode) .collect(Collectors.toSet()); + if (boundaryCodes.isEmpty()) { + log.warn("No boundaries found for hierarchy type: {}", hierarchyType); + } + return boundaryCodes; }health-services/resource-generator/src/main/java/org/egov/processor/service/ExcelParser.java (1)
Line range hint
76-91: Consider refactoring the constructor to reduce parameter count for better maintainabilityThe constructor has a large number of parameters, which can make the code difficult to read and maintain. Consider using a builder pattern or dependency injection framework (e.g., Spring's
@Autowired) to inject dependencies. This will improve readability and scalability of the class.health-services/plan-service/src/main/java/digit/web/models/AdditionalField.java (1)
Line range hint
1-44: Enhance field validation and documentation.
- The
valuefield type (BigDecimal) is restrictive. Consider using Object type to support different value types.- Add documentation for each field.
- Add validation for the
orderfield to ensure it's positive.@JsonProperty("value") @Valid @NotNull -private BigDecimal value = null; +private Object value = null; @JsonProperty("showOnUi") +@NotNull private Boolean showOnUi = Boolean.TRUE; @JsonProperty("editable") +@NotNull private Boolean editable = Boolean.TRUE; @JsonProperty("order") +@NotNull +@Min(1) private Integer order = null;Also, add Javadoc for each field:
/** * Unique identifier for the additional field. */ @JsonProperty("id") /** * Key identifier for the additional field. */ @JsonProperty("key") /** * Value of the additional field. Can be of any type. */ @JsonProperty("value") /** * Flag to control UI visibility. Defaults to true. */ @JsonProperty("showOnUi") /** * Flag to control editability. Defaults to true. */ @JsonProperty("editable") /** * Display order of the field. Must be positive. */ @JsonProperty("order")
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (32)
health-services/plan-service/src/main/java/digit/config/ServiceConstants.java(1 hunks)health-services/plan-service/src/main/java/digit/repository/impl/PlanRepositoryImpl.java(1 hunks)health-services/plan-service/src/main/java/digit/repository/querybuilder/PlanQueryBuilder.java(2 hunks)health-services/plan-service/src/main/java/digit/repository/rowmapper/PlanRowMapper.java(4 hunks)health-services/plan-service/src/main/java/digit/service/PlanEnricher.java(5 hunks)health-services/plan-service/src/main/java/digit/service/PlanService.java(1 hunks)health-services/plan-service/src/main/java/digit/service/PlanValidator.java(1 hunks)health-services/plan-service/src/main/java/digit/service/enrichment/PlanFacilityEnricher.java(1 hunks)health-services/plan-service/src/main/java/digit/service/validator/PlanEmployeeAssignmentValidator.java(1 hunks)health-services/plan-service/src/main/java/digit/service/validator/PlanFacilityValidator.java(1 hunks)health-services/plan-service/src/main/java/digit/web/models/AdditionalField.java(1 hunks)health-services/plan-service/src/main/java/digit/web/models/Plan.java(1 hunks)health-services/plan-service/src/main/java/digit/web/models/PlanDTO.java(1 hunks)health-services/plan-service/src/main/java/digit/web/models/PlanSearchCriteria.java(4 hunks)health-services/plan-service/src/main/java/digit/web/models/census/Census.java(1 hunks)health-services/plan-service/src/main/resources/db/migration/main/V20242112151500__plan_additional_field_create_ddl.sql(1 hunks)health-services/resource-generator/src/main/java/org/egov/processor/config/Configuration.java(1 hunks)health-services/resource-generator/src/main/java/org/egov/processor/config/ServiceConstants.java(5 hunks)health-services/resource-generator/src/main/java/org/egov/processor/service/ExcelParser.java(13 hunks)health-services/resource-generator/src/main/java/org/egov/processor/util/CalculationUtil.java(2 hunks)health-services/resource-generator/src/main/java/org/egov/processor/util/EnrichmentUtil.java(3 hunks)health-services/resource-generator/src/main/java/org/egov/processor/util/ExcelStylingUtil.java(1 hunks)health-services/resource-generator/src/main/java/org/egov/processor/util/MdmsUtil.java(5 hunks)health-services/resource-generator/src/main/java/org/egov/processor/util/OutputEstimationGenerationUtil.java(1 hunks)health-services/resource-generator/src/main/java/org/egov/processor/util/ParsingUtil.java(4 hunks)health-services/resource-generator/src/main/java/org/egov/processor/util/PlanFacilityUtil.java(1 hunks)health-services/resource-generator/src/main/java/org/egov/processor/util/PlanUtil.java(5 hunks)health-services/resource-generator/src/main/java/org/egov/processor/web/models/planFacility/PlanFacility.java(1 hunks)health-services/resource-generator/src/main/java/org/egov/processor/web/models/planFacility/PlanFacilityResponse.java(1 hunks)health-services/resource-generator/src/main/java/org/egov/processor/web/models/planFacility/PlanFacilitySearchCriteria.java(1 hunks)health-services/resource-generator/src/main/java/org/egov/processor/web/models/planFacility/PlanFacilitySearchRequest.java(1 hunks)health-services/resource-generator/src/main/resources/application.properties(1 hunks)
🧰 Additional context used
📓 Learnings (12)
health-services/plan-service/src/main/java/digit/web/models/PlanDTO.java (1)
Learnt from: Priyanka-eGov
PR: egovernments/health-campaign-services#737
File: health-services/plan-service/src/main/java/digit/web/models/Plan.java:46-47
Timestamp: 2024-11-10T20:01:49.000Z
Learning: The `additionalDetails` field in the `Plan` class can be empty or null, as clarified by Priyanka-eGov.
health-services/resource-generator/src/main/java/org/egov/processor/config/Configuration.java (1)
Learnt from: devdatta-egov
PR: egovernments/health-campaign-services#741
File: health-services/resource-estimation-service/src/main/java/org/egov/processor/config/Configuration.java:48-49
Timestamp: 2024-11-10T20:01:49.000Z
Learning: The `planCreateEndPoint` configuration property in `Configuration.java` is accessed via its getter method in `PlanUtil.java`.
health-services/plan-service/src/main/java/digit/web/models/Plan.java (1)
Learnt from: Priyanka-eGov
PR: egovernments/health-campaign-services#737
File: health-services/plan-service/src/main/java/digit/web/models/Plan.java:46-47
Timestamp: 2024-11-10T20:01:49.000Z
Learning: The `additionalDetails` field in the `Plan` class can be empty or null, as clarified by Priyanka-eGov.
health-services/plan-service/src/main/java/digit/service/enrichment/PlanFacilityEnricher.java (1)
Learnt from: Saloni-eGov
PR: egovernments/health-campaign-services#906
File: health-services/plan-service/src/main/java/digit/web/models/PlanFacility.java:61-67
Timestamp: 2024-11-12T10:40:11.591Z
Learning: In the `addServiceBoundariesItem` method in `PlanFacility.java`, it's acceptable to initialize `serviceBoundaries` if it's null without adding a null check for `serviceBoundariesItem`.
health-services/resource-generator/src/main/java/org/egov/processor/web/models/planFacility/PlanFacilityResponse.java (2)
Learnt from: Taniya-eGov
PR: egovernments/health-campaign-services#914
File: health-services/plan-service/src/main/java/digit/web/models/PlanFacilityResponse.java:37-43
Timestamp: 2024-11-10T20:01:49.000Z
Learning: In the `PlanFacilityResponse` class, the `addPlanFacilityItem` method is intended to be called only with valid `PlanFacility` objects, so adding null checks for `planFacilityItem` is unnecessary.
Learnt from: Saloni-eGov
PR: egovernments/health-campaign-services#906
File: health-services/plan-service/src/main/java/digit/web/models/PlanFacilityResponse.java:25-30
Timestamp: 2024-11-12T10:40:11.591Z
Learning: In this codebase, PascalCase is preferred for JSON property names in `@JsonProperty` annotations.
health-services/plan-service/src/main/resources/db/migration/main/V20242112151500__plan_additional_field_create_ddl.sql (1)
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-11-12T10:40:11.591Z
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.
health-services/resource-generator/src/main/java/org/egov/processor/util/CalculationUtil.java (1)
Learnt from: devdatta-egov
PR: egovernments/health-campaign-services#784
File: health-services/resource-estimation-service/src/main/java/org/egov/processor/util/CalculationUtil.java:74-87
Timestamp: 2024-11-10T20:01:49.000Z
Learning: The `calculateResources` method in `CalculationUtil.java` is intentionally complex to meet specific design requirements or constraints.
health-services/resource-generator/src/main/java/org/egov/processor/web/models/planFacility/PlanFacilitySearchRequest.java (1)
Learnt from: Taniya-eGov
PR: egovernments/health-campaign-services#914
File: health-services/plan-service/src/main/java/digit/web/models/facility/FacilitySearchRequest.java:15-19
Timestamp: 2024-11-10T20:01:49.000Z
Learning: In the `FacilitySearchRequest` class, the JSON property for `facilitySearchCriteria` should remain as `Facility` to maintain consistency with the existing API contract and avoid confusing existing API consumers.
health-services/plan-service/src/main/java/digit/service/PlanValidator.java (1)
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-11-12T10:40:11.591Z
Learning: The plan-service project is using Java 17.
health-services/resource-generator/src/main/java/org/egov/processor/web/models/planFacility/PlanFacility.java (3)
Learnt from: Saloni-eGov
PR: egovernments/health-campaign-services#906
File: health-services/plan-service/src/main/java/digit/web/models/PlanFacility.java:61-67
Timestamp: 2024-11-12T10:40:11.591Z
Learning: In the `addServiceBoundariesItem` method in `PlanFacility.java`, it's acceptable to initialize `serviceBoundaries` if it's null without adding a null check for `serviceBoundariesItem`.
Learnt from: Taniya-eGov
PR: egovernments/health-campaign-services#914
File: health-services/plan-service/src/main/java/digit/web/models/PlanFacility.java:1-69
Timestamp: 2024-11-12T10:40:11.591Z
Learning: In the `PlanFacility` model, the `active` field is set to `true` during creation, so it does not need to be initialized to `true` in the class definition.
Learnt from: Taniya-eGov
PR: egovernments/health-campaign-services#914
File: health-services/plan-service/src/main/java/digit/web/models/PlanFacilityResponse.java:37-43
Timestamp: 2024-11-10T20:01:49.000Z
Learning: In the `PlanFacilityResponse` class, the `addPlanFacilityItem` method is intended to be called only with valid `PlanFacility` objects, so adding null checks for `planFacilityItem` is unnecessary.
health-services/plan-service/src/main/java/digit/service/PlanEnricher.java (1)
Learnt from: Taniya-eGov
PR: egovernments/health-campaign-services#914
File: health-services/plan-service/src/main/java/digit/service/enrichment/PlanFacilityEnrichementService.java:0-0
Timestamp: 2024-11-12T10:40:17.779Z
Learning: The `active` status is intentionally set to true during creation to ensure that newly created plan facilities are active by default. Making it configurable might complicate the creation process and isn't necessary for current requirements.
health-services/resource-generator/src/main/resources/application.properties (1)
Learnt from: Priyanka-eGov
PR: egovernments/health-campaign-services#934
File: health-services/plan-service/src/main/resources/application.properties:54-54
Timestamp: 2024-11-12T10:40:17.779Z
Learning: In the 'health-campaign-services' project, it is acceptable to keep the old MDMS endpoint `/egov-mdms-service/v1/_search` in the following files, and updating them is not required:
- `health-services/referralmanagement/src/main/resources/application.properties`
- `health-services/transformer/src/main/resources/application.properties`
- `health-services/stock/src/main/resources/application.properties`
- `health-services/resource-estimation-service/src/main/resources/application.properties`
- `health-services/individual/src/main/resources/application.properties`
- `health-services/product/src/main/resources/application.properties`
- `health-services/project/src/main/resources/application.properties`
- `health-services/household/src/main/resources/application.properties`
- `health-services/facility/src/main/resources/application.properties`
- `core-services/pgr-services/src/main/resources/application.properties`
- `core-services/egov-hrms/src/main/resources/application.properties`
- `core-services/dashboard-analytics/src/main/resources/application.properties`
🔇 Additional comments (32)
health-services/plan-service/src/main/java/digit/service/validator/PlanFacilityValidator.java (1)
143-154: LGTM! Clean and efficient implementation.The method is well-documented and implements a straightforward mapping of boundaries to their codes.
health-services/resource-generator/src/main/java/org/egov/processor/web/models/planFacility/PlanFacilityResponse.java (3)
1-12: LGTM! Clean and well-organized imports.The package structure and imports follow standard conventions and include all necessary dependencies.
13-21: LGTM! Well-structured class with appropriate annotations.The class is properly documented and uses appropriate Lombok annotations for reducing boilerplate code while maintaining flexibility.
23-24: LGTM! Consistent JSON property naming.The PascalCase naming in @JsonProperty annotations follows the codebase conventions as confirmed by previous learnings.
Also applies to: 26-28, 30-32
health-services/resource-generator/src/main/java/org/egov/processor/config/Configuration.java (1)
44-46: LGTM! Verify the endpoint usage.The new configuration field for plan facility search endpoint is properly defined and follows the established pattern.
Let's verify if this endpoint is being used correctly:
✅ Verification successful
Endpoint configuration is properly implemented and used
The
planFacilitySearchEndPointconfiguration is actively used inPlanFacilityUtil.javato construct the search URI for plan facility operations. The implementation follows good practices with proper error handling and logging.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for usages of the new endpoint configuration rg -l "planFacilitySearchEndPoint" --type javaLength of output: 140
Script:
#!/bin/bash # Search for similar endpoint patterns and related classes rg -l "facility.*search" --type java # Search for plan-related service or utility classes fd -e java -e properties . | rg -i "plan|facility"Length of output: 25472
Script:
#!/bin/bash # Check PlanFacilityUtil implementation cat health-services/resource-generator/src/main/java/org/egov/processor/util/PlanFacilityUtil.javaLength of output: 2270
health-services/resource-generator/src/main/resources/application.properties (2)
62-65: LGTM! Plan config properties are properly configured.The plan config host and endpoint properties are correctly uncommented and follow the established pattern.
70-72: LGTM! Plan facility endpoint is properly configured.The new plan facility search endpoint follows REST API naming conventions and is properly organized with other plan-related configurations.
health-services/resource-generator/src/main/java/org/egov/processor/util/MdmsUtil.java (3)
30-31: LGTM! Clean dependency management.The static import and new field addition are well-organized and follow proper encapsulation principles.
Also applies to: 42-43
44-49: LGTM! Constructor follows dependency injection pattern.The constructor properly initializes all dependencies, including the new
ParsingUtil.
112-124: LGTM! Well-structured and documented method.The new method follows consistent patterns and is properly documented.
health-services/resource-generator/src/main/java/org/egov/processor/util/OutputEstimationGenerationUtil.java (1)
21-268: 🧹 Nitpick (assertive)Lack of Unit Tests for Utility Class
The
OutputEstimationGenerationUtilclass contains complex logic but lacks unit tests to validate its functionality.Consider adding unit tests to cover various scenarios and edge cases. Would you like assistance in creating a test suite for this class?
health-services/resource-generator/src/main/java/org/egov/processor/service/ExcelParser.java (1)
471-476: Verify the correctness of the header cell creation logicIn the
performCalculationsOnOperationsmethod, the conditionif (row.getRowNum() == 1)checks for the second row (since indexing starts at 0). However, header cells are typically on the first row (row.getRowNum() == 0). Ensure that this condition correctly identifies the header row, and adjust if necessary.health-services/plan-service/src/main/java/digit/service/PlanValidator.java (1)
1-1: Verify the impact of the package change on dependent code.Changing the package declaration from
digit.service.validatortodigit.servicemay affect other classes that importPlanValidator. Ensure that all imports and references toPlanValidatorare updated accordingly throughout the codebase to prevent anyClassNotFoundExceptionor similar issues.Run the following script to identify any lingering references to the old package:
health-services/plan-service/src/main/java/digit/web/models/PlanSearchCriteria.java (1)
43-45: Addition offacilityIdenhances search capabilities.Including
facilityIdin the search criteria allows for more specific queries and aligns with the search functionality enhancement.health-services/resource-generator/src/main/java/org/egov/processor/web/models/planFacility/PlanFacilitySearchCriteria.java (1)
26-28: LGTM! Proper validation for tenantId.The
@NotNullconstraint ontenantIdensures this required field is always provided.health-services/plan-service/src/main/java/digit/web/models/Plan.java (1)
77-79: LGTM! Proper implementation of additional fields.The new
additionalFieldsproperty is correctly implemented with proper validation and JSON annotations, consistent with the existing code style.health-services/plan-service/src/main/java/digit/web/models/PlanDTO.java (1)
62-64: LGTM! Consistent implementation with Plan class.The
additionalFieldsproperty mirrors the implementation in the Plan class, maintaining consistency across the codebase.health-services/resource-generator/src/main/java/org/egov/processor/web/models/planFacility/PlanFacility.java (1)
82-88: LGTM! Method implementation aligns with past feedback.The
addServiceBoundariesItemmethod correctly initializes the list if null, as confirmed by the learning from PR #906.health-services/plan-service/src/main/java/digit/web/models/census/Census.java (1)
11-11: LGTM! Additional fields support added correctly.The addition of
additionalFieldswith proper validation annotations enhances the model's flexibility while maintaining data integrity.Also applies to: 62-63
health-services/plan-service/src/main/java/digit/service/PlanService.java (1)
62-64: Verify the impact of search request enrichment.The addition of
planEnricher.enrichSearchRequest(body)modifies the search behavior. Ensure that:
- The enrichment doesn't affect existing search functionality
- The search results remain consistent with previous behavior
✅ Verification successful
Search request enrichment is safe and maintains existing behavior
The enrichment only adds optional facility-related filters when a facility ID is present in the search criteria. It doesn't modify any existing search parameters or affect the core search functionality.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for any existing search criteria handling rg -A 5 "PlanSearchCriteria" . # Check for any test cases related to plan search fd -e java -x rg -l "test.*Plan.*Search" {}Length of output: 21551
Script:
#!/bin/bash # Find enrichSearchRequest implementation ast-grep --pattern 'enrichSearchRequest($$$) { $$$ }' # Look for enrichment related files rg -A 10 "class.*PlanEnricher" . # Check for any enrichment configuration rg -l "enrich.*config" .Length of output: 1527
Script:
#!/bin/bash # Find the enrichSearchRequest method implementation rg -A 15 "enrichSearchRequest.*\{" . # Find any references to this method rg "enrichSearchRequest\(" . # Look for search request modifications rg -A 10 "PlanSearchRequest.*build" .Length of output: 9809
health-services/resource-generator/src/main/java/org/egov/processor/util/CalculationUtil.java (1)
82-82: Verify the impact of passing null to planUtil.create.The change from
mappedValuestonullin theplanUtil.createcall might affect the behavior of resource calculation.✅ Verification successful
Passing null to planUtil.create is safe and intentional
The implementation of
PlanUtil.create()explicitly handles null input for theboundaryCodeToCensusAdditionalDetailsparameter. When null is passed, it safely processes the plan without census additional details, which is an acceptable use case in the resource calculation context.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check the implementation of planUtil.create ast-grep --pattern 'create($$$, $$$, $$$, $$$, $$$) { $$$ }' # Check for any test cases that might be affected fd -e java -x rg -l "test.*planUtil.*create" {}Length of output: 116
Script:
#!/bin/bash # Find PlanUtil class and its implementation ast-grep --pattern 'class PlanUtil { $$$ }' # Try a more flexible pattern for create method ast-grep --pattern 'create($_, $_, $_, $_, $_)' # Search for test files and usages rg -l "class.*PlanUtil" --type java rg "planUtil.create" -A 5 -B 5Length of output: 3222
Script:
#!/bin/bash # Get the content of PlanUtil.java to see the implementation cat health-services/resource-generator/src/main/java/org/egov/processor/util/PlanUtil.java # Look for any related configuration or model classes rg -l "BoundaryCodeToCensusAdditionalDetails" --type javaLength of output: 8696
health-services/plan-service/src/main/java/digit/repository/impl/PlanRepositoryImpl.java (1)
192-192: LGTM!The addition of
additionalFieldsto the DTO builder is consistent with the changes to support additional fields in the plan service.health-services/plan-service/src/main/java/digit/service/PlanEnricher.java (2)
57-60: LGTM!The UUID enrichment for additional fields follows the same pattern as other entities and includes proper null checks.
115-121: LGTM!The enrichSearchRequest implementation is clean and well-structured:
- Proper handling of facility ID filtering
- Good use of constants
- Appropriate null checks
Also applies to: 244-263
health-services/plan-service/src/main/java/digit/repository/rowmapper/PlanRowMapper.java (1)
33-33: LGTM!The implementation of additional fields handling in the row mapper is well-structured:
- Proper tracking of processed fields using a Set
- Well-documented method with clear Javadoc
- Consistent with the pattern used for other entities
- Robust null checks and list initialization
Also applies to: 47-47, 77-77, 241-273
health-services/plan-service/src/main/java/digit/repository/querybuilder/PlanQueryBuilder.java (1)
33-34: SQL query changes look good!The additional fields are properly integrated into the query with appropriate aliases, and the LEFT JOIN ensures backward compatibility for plans without additional fields.
Also applies to: 39-40
health-services/resource-generator/src/main/java/org/egov/processor/util/EnrichmentUtil.java (1)
225-225: Documentation updates are accurate.Comments correctly reflect the context change from census to plan records.
Also applies to: 228-228
health-services/resource-generator/src/main/java/org/egov/processor/util/ParsingUtil.java (1)
394-429: Well-implemented JSON field extraction!The implementation handles different JSON types comprehensively with proper error handling and logging.
health-services/plan-service/src/main/java/digit/config/ServiceConstants.java (1)
363-364: Constant addition follows conventions.The new facility ID search parameter constant is well-placed and follows the established naming pattern.
health-services/plan-service/src/main/java/digit/service/validator/PlanEmployeeAssignmentValidator.java (1)
320-322: LGTM! Good optimization.The change to validate campaign details only for active records is a good optimization that avoids unnecessary validations.
health-services/resource-generator/src/main/java/org/egov/processor/util/PlanUtil.java (2)
35-42: LGTM! Constructor dependency injection properly implemented.The addition of
ParsingUtildependency follows good practices with proper constructor injection.
77-78: LGTM! Clean integration of additional details enrichment.The changes maintain good separation of concerns and the builder pattern is properly utilized.
Also applies to: 96-98
| Set<String> lowestHierarchyBCodes = fetchBoundaryCodes(campaignResponse.getCampaignDetails().get(0), lowestHierarchy); | ||
| Set<String> allBoundaryCodes = fetchAllBoundaryCodes(campaignResponse.getCampaignDetails().get(0)); | ||
|
|
||
| // Validate residing boundaries | ||
| validateResidingBoundaries(boundaryCodes, planFacility); | ||
| // Validate residing boundaries with all the boundary codes. | ||
| validateResidingBoundaries(allBoundaryCodes, planFacility); | ||
|
|
||
| // Validate service boundaries | ||
| validateServiceBoundaries(boundaryCodes, planFacility); | ||
| // Validate service boundaries with the lowest hierarchy boundary codes. | ||
| validateServiceBoundaries(lowestHierarchyBCodes, planFacility); |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Document the rationale for separate boundary validations.
While the code structure is good, please add comments explaining why:
- Residing boundaries are validated against all boundary codes
- Service boundaries are validated against only the lowest hierarchy boundary codes
This will help future maintainers understand the business logic behind these distinct validations.
|
|
||
| @JsonProperty("ResponseInfo") | ||
| private ResponseInfo responseInfo = null; | ||
|
|
||
| @JsonProperty("PlanFacility") | ||
| @Valid | ||
| private List<PlanFacility> planFacility = null; | ||
|
|
||
| @JsonProperty("TotalCount") | ||
| @Valid | ||
| private Integer totalCount = null; | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Consider enhancing field documentation and removing redundant null initialization.
- Consider adding field-level documentation to describe the purpose of each field.
- The explicit null initialization is redundant as fields are automatically initialized to null in Java.
public class PlanFacilityResponse {
+ /**
+ * Response info object containing metadata about the API response
+ */
@JsonProperty("ResponseInfo")
- private ResponseInfo responseInfo = null;
+ private ResponseInfo responseInfo;
+ /**
+ * List of plan facilities matching the search criteria
+ */
@JsonProperty("PlanFacility")
@Valid
- private List<PlanFacility> planFacility = null;
+ private List<PlanFacility> planFacility;
+ /**
+ * Total count of plan facilities matching the search criteria
+ */
@JsonProperty("TotalCount")
@Valid
- private Integer totalCount = null;
+ private Integer totalCount;
}📝 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") | |
| private ResponseInfo responseInfo = null; | |
| @JsonProperty("PlanFacility") | |
| @Valid | |
| private List<PlanFacility> planFacility = null; | |
| @JsonProperty("TotalCount") | |
| @Valid | |
| private Integer totalCount = null; | |
| } | |
| /** | |
| * Response info object containing metadata about the API response | |
| */ | |
| @JsonProperty("ResponseInfo") | |
| private ResponseInfo responseInfo; | |
| /** | |
| * List of plan facilities matching the search criteria | |
| */ | |
| @JsonProperty("PlanFacility") | |
| @Valid | |
| private List<PlanFacility> planFacility; | |
| /** | |
| * Total count of plan facilities matching the search criteria | |
| */ | |
| @JsonProperty("TotalCount") | |
| @Valid | |
| private Integer totalCount; | |
| } |
health-services/resource-generator/src/main/java/org/egov/processor/util/MdmsUtil.java
Show resolved
Hide resolved
| public Map<String, Object> filterMasterData(String masterDataJson, String campaignType) { | ||
| Map<String, Object> properties = new HashMap<>(); | ||
| Map<String, Object> masterData = JsonUtils.parseJson(masterDataJson, Map.class); | ||
| Map<String, Object> planModule = (Map<String, Object>) masterData.get(ServiceConstants.MDMS_PLAN_MODULE_NAME); | ||
| List<Map<String, Object>> schemas = (List<Map<String, Object>>) planModule | ||
| .get(ServiceConstants.MDMS_MASTER_SCHEMAS); | ||
| log.info("masterDataJson ==>" + schemas); | ||
| for (Map<String, Object> schema : schemas) { | ||
| String type = (String) schema.get(ServiceConstants.MDMS_SCHEMA_TYPE); | ||
| Map<String, Object> adminConsoleModule = (Map<String, Object>) masterData.get(ServiceConstants.MDMS_ADMIN_CONSOLE_MODULE_NAME); | ||
| List<Map<String, Object>> adminSchema = (List<Map<String, Object>>) adminConsoleModule | ||
| .get(ServiceConstants.MDMS_MASTER_ADMIN_SCHEMA); | ||
| log.debug("masterDataJson ==> " + adminSchema); | ||
|
|
||
| for (Map<String, Object> schema : adminSchema) { | ||
| String campaign = (String) schema.get(ServiceConstants.MDMS_CAMPAIGN_TYPE); | ||
| // String fileT = InputFileTypeEnum.valueOf(type); | ||
| if (schema.get(ServiceConstants.MDMS_SCHEMA_SECTION).equals(ServiceConstants.FILE_TEMPLATE_IDENTIFIER_POPULATION) | ||
| && campaign.equals(campaignType) && type.equals(fileType.toString())) { | ||
| Map<String, Object> schemaProperties = (Map<String, Object>) schema.get("schema"); | ||
| properties = (Map<String, Object>) schemaProperties.get("Properties"); | ||
|
|
||
| // Check if the schema's title matches the required template identifier | ||
| // and the campaign matches the specified campaign type. | ||
| if (schema.get(ServiceConstants.MDMS_SCHEMA_TITLE).equals(ServiceConstants.FILE_TEMPLATE_IDENTIFIER_BOUNDARY) | ||
| && campaign.equals(MICROPLAN_PREFIX + campaignType)) { | ||
| Map<String, List<Object>> schemaProperties = (Map<String, List<Object>>) schema.get("properties"); | ||
|
|
||
| schemaProperties.forEach((propertyType, propertyList) -> | ||
| // For each property in the property list, extract its name and add it to the map with the property. | ||
| propertyList.forEach(property -> { | ||
| String propertyName = (String) parsingUtil.extractFieldsFromJsonObject(property, "name"); | ||
| properties.put(propertyName, property); | ||
| }) | ||
| ); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add comprehensive error handling for data processing.
The method processes complex nested data structures but lacks proper error handling. Consider these improvements:
public Map<String, Object> filterMasterData(String masterDataJson, String campaignType) {
Map<String, Object> properties = new HashMap<>();
+ if (masterDataJson == null || campaignType == null) {
+ throw new CustomException("INVALID_INPUT", "masterDataJson and campaignType cannot be null");
+ }
+ try {
Map<String, Object> masterData = JsonUtils.parseJson(masterDataJson, Map.class);
Map<String, Object> adminConsoleModule = (Map<String, Object>) masterData.get(ServiceConstants.MDMS_ADMIN_CONSOLE_MODULE_NAME);
+ if (adminConsoleModule == null) {
+ throw new CustomException("INVALID_MODULE", "Admin console module not found in master data");
+ }
List<Map<String, Object>> adminSchema = (List<Map<String, Object>>) adminConsoleModule
.get(ServiceConstants.MDMS_MASTER_ADMIN_SCHEMA);
+ if (adminSchema == null) {
+ throw new CustomException("INVALID_SCHEMA", "Admin schema not found in master data");
+ }
log.debug("masterDataJson ==> " + adminSchema);
for (Map<String, Object> schema : adminSchema) {
String campaign = (String) schema.get(ServiceConstants.MDMS_CAMPAIGN_TYPE);
+ if (campaign == null || schema.get(ServiceConstants.MDMS_SCHEMA_TITLE) == null) {
+ log.warn("Skipping schema with missing required fields");
+ continue;
+ }
if (schema.get(ServiceConstants.MDMS_SCHEMA_TITLE).equals(ServiceConstants.FILE_TEMPLATE_IDENTIFIER_BOUNDARY)
&& campaign.equals(MICROPLAN_PREFIX + campaignType)) {
Map<String, List<Object>> schemaProperties = (Map<String, List<Object>>) schema.get("properties");
+ if (schemaProperties == null) {
+ log.warn("Schema properties not found for campaign type: {}", campaignType);
+ continue;
+ }
schemaProperties.forEach((propertyType, propertyList) ->
propertyList.forEach(property -> {
+ try {
String propertyName = (String) parsingUtil.extractFieldsFromJsonObject(property, "name");
+ if (propertyName != null) {
properties.put(propertyName, property);
+ }
+ } catch (Exception e) {
+ log.error("Error processing property: {}", e.getMessage());
+ }
})
);
}
}
+ } catch (Exception e) {
+ log.error("Error filtering master data: {}", e.getMessage());
+ throw new CustomException("PROCESSING_ERROR", "Error processing master data: " + e.getMessage());
+ }
return properties;
}📝 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<String, Object> filterMasterData(String masterDataJson, String campaignType) { | |
| Map<String, Object> properties = new HashMap<>(); | |
| Map<String, Object> masterData = JsonUtils.parseJson(masterDataJson, Map.class); | |
| Map<String, Object> planModule = (Map<String, Object>) masterData.get(ServiceConstants.MDMS_PLAN_MODULE_NAME); | |
| List<Map<String, Object>> schemas = (List<Map<String, Object>>) planModule | |
| .get(ServiceConstants.MDMS_MASTER_SCHEMAS); | |
| log.info("masterDataJson ==>" + schemas); | |
| for (Map<String, Object> schema : schemas) { | |
| String type = (String) schema.get(ServiceConstants.MDMS_SCHEMA_TYPE); | |
| Map<String, Object> adminConsoleModule = (Map<String, Object>) masterData.get(ServiceConstants.MDMS_ADMIN_CONSOLE_MODULE_NAME); | |
| List<Map<String, Object>> adminSchema = (List<Map<String, Object>>) adminConsoleModule | |
| .get(ServiceConstants.MDMS_MASTER_ADMIN_SCHEMA); | |
| log.debug("masterDataJson ==> " + adminSchema); | |
| for (Map<String, Object> schema : adminSchema) { | |
| String campaign = (String) schema.get(ServiceConstants.MDMS_CAMPAIGN_TYPE); | |
| // String fileT = InputFileTypeEnum.valueOf(type); | |
| if (schema.get(ServiceConstants.MDMS_SCHEMA_SECTION).equals(ServiceConstants.FILE_TEMPLATE_IDENTIFIER_POPULATION) | |
| && campaign.equals(campaignType) && type.equals(fileType.toString())) { | |
| Map<String, Object> schemaProperties = (Map<String, Object>) schema.get("schema"); | |
| properties = (Map<String, Object>) schemaProperties.get("Properties"); | |
| // Check if the schema's title matches the required template identifier | |
| // and the campaign matches the specified campaign type. | |
| if (schema.get(ServiceConstants.MDMS_SCHEMA_TITLE).equals(ServiceConstants.FILE_TEMPLATE_IDENTIFIER_BOUNDARY) | |
| && campaign.equals(MICROPLAN_PREFIX + campaignType)) { | |
| Map<String, List<Object>> schemaProperties = (Map<String, List<Object>>) schema.get("properties"); | |
| schemaProperties.forEach((propertyType, propertyList) -> | |
| // For each property in the property list, extract its name and add it to the map with the property. | |
| propertyList.forEach(property -> { | |
| String propertyName = (String) parsingUtil.extractFieldsFromJsonObject(property, "name"); | |
| properties.put(propertyName, property); | |
| }) | |
| ); | |
| public Map<String, Object> filterMasterData(String masterDataJson, String campaignType) { | |
| Map<String, Object> properties = new HashMap<>(); | |
| if (masterDataJson == null || campaignType == null) { | |
| throw new CustomException("INVALID_INPUT", "masterDataJson and campaignType cannot be null"); | |
| } | |
| try { | |
| Map<String, Object> masterData = JsonUtils.parseJson(masterDataJson, Map.class); | |
| Map<String, Object> adminConsoleModule = (Map<String, Object>) masterData.get(ServiceConstants.MDMS_ADMIN_CONSOLE_MODULE_NAME); | |
| if (adminConsoleModule == null) { | |
| throw new CustomException("INVALID_MODULE", "Admin console module not found in master data"); | |
| } | |
| List<Map<String, Object>> adminSchema = (List<Map<String, Object>>) adminConsoleModule | |
| .get(ServiceConstants.MDMS_MASTER_ADMIN_SCHEMA); | |
| if (adminSchema == null) { | |
| throw new CustomException("INVALID_SCHEMA", "Admin schema not found in master data"); | |
| } | |
| log.debug("masterDataJson ==> " + adminSchema); | |
| for (Map<String, Object> schema : adminSchema) { | |
| String campaign = (String) schema.get(ServiceConstants.MDMS_CAMPAIGN_TYPE); | |
| if (campaign == null || schema.get(ServiceConstants.MDMS_SCHEMA_TITLE) == null) { | |
| log.warn("Skipping schema with missing required fields"); | |
| continue; | |
| } | |
| if (schema.get(ServiceConstants.MDMS_SCHEMA_TITLE).equals(ServiceConstants.FILE_TEMPLATE_IDENTIFIER_BOUNDARY) | |
| && campaign.equals(MICROPLAN_PREFIX + campaignType)) { | |
| Map<String, List<Object>> schemaProperties = (Map<String, List<Object>>) schema.get("properties"); | |
| if (schemaProperties == null) { | |
| log.warn("Schema properties not found for campaign type: {}", campaignType); | |
| continue; | |
| } | |
| schemaProperties.forEach((propertyType, propertyList) -> | |
| propertyList.forEach(property -> { | |
| try { | |
| String propertyName = (String) parsingUtil.extractFieldsFromJsonObject(property, "name"); | |
| if (propertyName != null) { | |
| properties.put(propertyName, property); | |
| } | |
| } catch (Exception e) { | |
| log.error("Error processing property: {}", e.getMessage()); | |
| } | |
| }) | |
| ); | |
| } | |
| } | |
| } catch (Exception e) { | |
| log.error("Error filtering master data: {}", e.getMessage()); | |
| throw new CustomException("PROCESSING_ERROR", "Error processing master data: " + e.getMessage()); | |
| } | |
| return properties; | |
| } |
...resource-generator/src/main/java/org/egov/processor/util/OutputEstimationGenerationUtil.java
Show resolved
Hide resolved
| /** | ||
| * Adds or updates the value of provided field in the additional details object. | ||
| * @param additionalDetails | ||
| * @param fieldsToBeUpdated | ||
| * @return | ||
| */ | ||
| public Map<String, Object> updateFieldInAdditionalDetails(Object additionalDetails, Map<String, Object> fieldsToBeUpdated) { | ||
| try { | ||
|
|
||
| // Get or create the additionalDetails as an ObjectNode | ||
| ObjectNode objectNode = (additionalDetails == null || additionalDetails instanceof NullNode) | ||
| ? objectMapper.createObjectNode() | ||
| : objectMapper.convertValue(additionalDetails, ObjectNode.class); | ||
|
|
||
| // Update or add the field in additional details object | ||
| fieldsToBeUpdated.forEach((key, value) -> objectNode.set(key, objectMapper.valueToTree(value))); | ||
|
|
||
| // Convert updated ObjectNode back to a Map | ||
| return objectMapper.convertValue(objectNode, Map.class); | ||
|
|
||
| } catch (Exception e) { | ||
| throw new CustomException(ERROR_WHILE_UPDATING_ADDITIONAL_DETAILS_CODE, ERROR_WHILE_UPDATING_ADDITIONAL_DETAILS_MESSAGE + e); | ||
| } | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Solid implementation with good null handling.
The implementation safely handles JSON updates with proper error handling. Consider adding debug logging for successful updates.
// Convert updated ObjectNode back to a Map
Map<String, Object> result = objectMapper.convertValue(objectNode, Map.class);
+log.debug("Successfully updated additional details for fields: {}", fieldsToBeUpdated.keySet());
return result;Committable suggestion skipped: line range outside the PR's diff.
| -- Table: plan_additional_field | ||
| CREATE TABLE plan_additional_field ( | ||
| id character varying(64) NOT NULL, | ||
| plan_id character varying(64) NOT NULL, | ||
| "key" character varying(64) NOT NULL, | ||
| "value" numeric(12,2) NOT NULL, | ||
| show_on_ui boolean DEFAULT true NOT NULL, | ||
| editable boolean DEFAULT true NOT NULL, | ||
| "order" bigint NOT NULL, | ||
| CONSTRAINT uk_plan_additional_field_id PRIMARY KEY (id), | ||
| FOREIGN KEY (plan_id) REFERENCES plan(id) | ||
| ); |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Add missing indexes and constraints for better performance and data integrity.
Consider the following improvements:
- Add an index on the foreign key column
plan_idfor better JOIN performance. - Add an index on the
keycolumn for efficient lookups. - Add a unique constraint on
(plan_id, key)to prevent duplicate keys for the same plan. - Consider using JSONB for the
valuecolumn instead of numeric(12,2) to support different value types.
CREATE TABLE plan_additional_field (
id character varying(64) NOT NULL,
plan_id character varying(64) NOT NULL,
"key" character varying(64) NOT NULL,
- "value" numeric(12,2) NOT NULL,
+ "value" jsonb NOT NULL,
show_on_ui boolean DEFAULT true NOT NULL,
editable boolean DEFAULT true NOT NULL,
"order" bigint NOT NULL,
CONSTRAINT uk_plan_additional_field_id PRIMARY KEY (id),
- FOREIGN KEY (plan_id) REFERENCES plan(id)
+ CONSTRAINT fk_plan_additional_field_plan FOREIGN KEY (plan_id) REFERENCES plan(id),
+ CONSTRAINT uk_plan_additional_field_plan_key UNIQUE (plan_id, "key")
);
+
+CREATE INDEX idx_plan_additional_field_plan_id ON plan_additional_field(plan_id);
+CREATE INDEX idx_plan_additional_field_key ON plan_additional_field("key");📝 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.
| -- Table: plan_additional_field | |
| CREATE TABLE plan_additional_field ( | |
| id character varying(64) NOT NULL, | |
| plan_id character varying(64) NOT NULL, | |
| "key" character varying(64) NOT NULL, | |
| "value" numeric(12,2) NOT NULL, | |
| show_on_ui boolean DEFAULT true NOT NULL, | |
| editable boolean DEFAULT true NOT NULL, | |
| "order" bigint NOT NULL, | |
| CONSTRAINT uk_plan_additional_field_id PRIMARY KEY (id), | |
| FOREIGN KEY (plan_id) REFERENCES plan(id) | |
| ); | |
| -- Table: plan_additional_field | |
| CREATE TABLE plan_additional_field ( | |
| id character varying(64) NOT NULL, | |
| plan_id character varying(64) NOT NULL, | |
| "key" character varying(64) NOT NULL, | |
| "value" jsonb NOT NULL, | |
| show_on_ui boolean DEFAULT true NOT NULL, | |
| editable boolean DEFAULT true NOT NULL, | |
| "order" bigint NOT NULL, | |
| CONSTRAINT uk_plan_additional_field_id PRIMARY KEY (id), | |
| CONSTRAINT fk_plan_additional_field_plan FOREIGN KEY (plan_id) REFERENCES plan(id), | |
| CONSTRAINT uk_plan_additional_field_plan_key UNIQUE (plan_id, "key") | |
| ); | |
| CREATE INDEX idx_plan_additional_field_plan_id ON plan_additional_field(plan_id); | |
| CREATE INDEX idx_plan_additional_field_key ON plan_additional_field("key"); |
| import digit.web.models.PlanFacilityRequest; | ||
| import digit.web.models.PlanFacilitySearchCriteria; | ||
| import digit.web.models.PlanFacilitySearchRequest; | ||
| import digit.web.models.*; |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Replace wildcard import with specific imports.
Wildcard imports can lead to naming conflicts and make it harder to track dependencies. Replace import digit.web.models.*; with specific imports for better maintainability.
| Map<String, BigDecimal> resultMap, Map<String, String> mappedValues, Map<String, Object> boundaryCodeToCensusAdditionalDetails) { | ||
| PlanRequest planRequest = buildPlanRequest(planConfigurationRequest, feature, resultMap, mappedValues, boundaryCodeToCensusAdditionalDetails); |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Update JavaDoc to include the new parameter.
The JavaDoc comments for both create and buildPlanRequest methods need to be updated to include documentation for the boundaryCodeToCensusAdditionalDetails parameter.
Also applies to: 74-74
| private Object enrichAdditionalDetials(Map<String, Object> boundaryCodeToCensusAdditionalDetails, String boundaryCodeValue) { | ||
| if(!CollectionUtils.isEmpty(boundaryCodeToCensusAdditionalDetails)) { | ||
|
|
||
| Object censusAdditionalDetails = boundaryCodeToCensusAdditionalDetails.get(boundaryCodeValue); | ||
|
|
||
| // Extract required details from census additional details object. | ||
| String facilityId = (String) parsingUtil.extractFieldsFromJsonObject(censusAdditionalDetails, FACILITY_ID); | ||
| Object accessibilityDetails = (Object) parsingUtil.extractFieldsFromJsonObject(censusAdditionalDetails, ACCESSIBILITY_DETAILS); | ||
| Object securityDetials = (Object) parsingUtil.extractFieldsFromJsonObject(censusAdditionalDetails, SECURITY_DETAILS); | ||
|
|
||
| // Creating a map of fields to be added in plan additional details with their key. | ||
| Map<String, Object> fieldsToBeUpdated = new HashMap<>(); | ||
| if(facilityId != null && !facilityId.isEmpty()) | ||
| fieldsToBeUpdated.put(FACILITY_ID, facilityId); | ||
|
|
||
| if(accessibilityDetails != null) | ||
| fieldsToBeUpdated.put(ACCESSIBILITY_DETAILS, accessibilityDetails); | ||
|
|
||
| if(securityDetials != null) | ||
| fieldsToBeUpdated.put(SECURITY_DETAILS, securityDetials); | ||
|
|
||
| if(!CollectionUtils.isEmpty(fieldsToBeUpdated)) | ||
| return parsingUtil.updateFieldInAdditionalDetails(new Object(), fieldsToBeUpdated); | ||
| } | ||
| return null; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fix typos and add null checks in enrichment method.
Several issues need attention:
- Method name has a typo:
enrichAdditionalDetials→enrichAdditionalDetails - Variable name has a typo:
securityDetials→securityDetails - Add null check for
censusAdditionalDetailsbefore extraction - Consider moving magic strings to constants
Here's the suggested fix:
- private Object enrichAdditionalDetials(Map<String, Object> boundaryCodeToCensusAdditionalDetails, String boundaryCodeValue) {
+ private Object enrichAdditionalDetails(Map<String, Object> boundaryCodeToCensusAdditionalDetails, String boundaryCodeValue) {
if(!CollectionUtils.isEmpty(boundaryCodeToCensusAdditionalDetails)) {
Object censusAdditionalDetails = boundaryCodeToCensusAdditionalDetails.get(boundaryCodeValue);
+ if (censusAdditionalDetails == null) {
+ return null;
+ }
// Extract required details from census additional details object.
String facilityId = (String) parsingUtil.extractFieldsFromJsonObject(censusAdditionalDetails, FACILITY_ID);
Object accessibilityDetails = (Object) parsingUtil.extractFieldsFromJsonObject(censusAdditionalDetails, ACCESSIBILITY_DETAILS);
- Object securityDetials = (Object) parsingUtil.extractFieldsFromJsonObject(censusAdditionalDetails, SECURITY_DETAILS);
+ Object securityDetails = (Object) parsingUtil.extractFieldsFromJsonObject(censusAdditionalDetails, SECURITY_DETAILS);
// Creating a map of fields to be added in plan additional details with their key.
Map<String, Object> fieldsToBeUpdated = new HashMap<>();
if(facilityId != null && !facilityId.isEmpty())
fieldsToBeUpdated.put(FACILITY_ID, facilityId);
if(accessibilityDetails != null)
fieldsToBeUpdated.put(ACCESSIBILITY_DETAILS, accessibilityDetails);
- if(securityDetials != null)
- fieldsToBeUpdated.put(SECURITY_DETAILS, securityDetials);
+ if(securityDetails != null)
+ fieldsToBeUpdated.put(SECURITY_DETAILS, securityDetails);
if(!CollectionUtils.isEmpty(fieldsToBeUpdated))
return parsingUtil.updateFieldInAdditionalDetails(new Object(), fieldsToBeUpdated);
}
return 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.
| private Object enrichAdditionalDetials(Map<String, Object> boundaryCodeToCensusAdditionalDetails, String boundaryCodeValue) { | |
| if(!CollectionUtils.isEmpty(boundaryCodeToCensusAdditionalDetails)) { | |
| Object censusAdditionalDetails = boundaryCodeToCensusAdditionalDetails.get(boundaryCodeValue); | |
| // Extract required details from census additional details object. | |
| String facilityId = (String) parsingUtil.extractFieldsFromJsonObject(censusAdditionalDetails, FACILITY_ID); | |
| Object accessibilityDetails = (Object) parsingUtil.extractFieldsFromJsonObject(censusAdditionalDetails, ACCESSIBILITY_DETAILS); | |
| Object securityDetials = (Object) parsingUtil.extractFieldsFromJsonObject(censusAdditionalDetails, SECURITY_DETAILS); | |
| // Creating a map of fields to be added in plan additional details with their key. | |
| Map<String, Object> fieldsToBeUpdated = new HashMap<>(); | |
| if(facilityId != null && !facilityId.isEmpty()) | |
| fieldsToBeUpdated.put(FACILITY_ID, facilityId); | |
| if(accessibilityDetails != null) | |
| fieldsToBeUpdated.put(ACCESSIBILITY_DETAILS, accessibilityDetails); | |
| if(securityDetials != null) | |
| fieldsToBeUpdated.put(SECURITY_DETAILS, securityDetials); | |
| if(!CollectionUtils.isEmpty(fieldsToBeUpdated)) | |
| return parsingUtil.updateFieldInAdditionalDetails(new Object(), fieldsToBeUpdated); | |
| } | |
| return null; | |
| private Object enrichAdditionalDetails(Map<String, Object> boundaryCodeToCensusAdditionalDetails, String boundaryCodeValue) { | |
| if(!CollectionUtils.isEmpty(boundaryCodeToCensusAdditionalDetails)) { | |
| Object censusAdditionalDetails = boundaryCodeToCensusAdditionalDetails.get(boundaryCodeValue); | |
| if (censusAdditionalDetails == null) { | |
| return null; | |
| } | |
| // Extract required details from census additional details object. | |
| String facilityId = (String) parsingUtil.extractFieldsFromJsonObject(censusAdditionalDetails, FACILITY_ID); | |
| Object accessibilityDetails = (Object) parsingUtil.extractFieldsFromJsonObject(censusAdditionalDetails, ACCESSIBILITY_DETAILS); | |
| Object securityDetails = (Object) parsingUtil.extractFieldsFromJsonObject(censusAdditionalDetails, SECURITY_DETAILS); | |
| // Creating a map of fields to be added in plan additional details with their key. | |
| Map<String, Object> fieldsToBeUpdated = new HashMap<>(); | |
| if(facilityId != null && !facilityId.isEmpty()) | |
| fieldsToBeUpdated.put(FACILITY_ID, facilityId); | |
| if(accessibilityDetails != null) | |
| fieldsToBeUpdated.put(ACCESSIBILITY_DETAILS, accessibilityDetails); | |
| if(securityDetails != null) | |
| fieldsToBeUpdated.put(SECURITY_DETAILS, securityDetails); | |
| if(!CollectionUtils.isEmpty(fieldsToBeUpdated)) | |
| return parsingUtil.updateFieldInAdditionalDetails(new Object(), fieldsToBeUpdated); | |
| } | |
| return null; |
Priyanka-eGov
changed the title
Priyanka-eGov
changed the title
Summary by CodeRabbit
Based on the comprehensive changes, here are the release notes:
New Features
Improvements
Database Changes
plan_additional_fieldtable to store flexible plan-related metadataTechnical Enhancements
These changes provide more flexibility in plan configuration and metadata management while enhancing the overall system's capabilities for resource generation and facility tracking.