HCMPRE-2701: Creating root boundary node #1567
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThis change introduces the initial implementation of the Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant GeopodeApiController
participant GeopodeAdapterService
participant ArcgisUtil
participant BoundaryUtil
participant MdmsUtil
participant ExternalServices
Client->>GeopodeApiController: POST /boundary/setup (GeopodeBoundaryRequest)
GeopodeApiController->>GeopodeAdapterService: createRootBoundaryData(request)
GeopodeAdapterService->>ArcgisUtil: createRoot(request)
ArcgisUtil->>MdmsUtil: fetchMdmsData(...)
MdmsUtil->>ExternalServices: POST MDMS endpoint
ExternalServices-->>MdmsUtil: MDMS data
ArcgisUtil->>ExternalServices: GET ArcGIS endpoint
ExternalServices-->>ArcgisUtil: ArcGIS data
ArcgisUtil->>BoundaryUtil: createBoundaryHierarchy(...)
BoundaryUtil->>ExternalServices: POST boundary hierarchy create endpoint
ExternalServices-->>BoundaryUtil: Boundary hierarchy response
ArcgisUtil-->>GeopodeAdapterService: BoundaryResponse
GeopodeAdapterService-->>GeopodeApiController: BoundaryResponse
GeopodeApiController-->>Client: 202 Accepted + BoundaryResponse
Suggested reviewers
Poem
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 review |
✅ Actions performedReview triggered.
|
|
@coderabbitai 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: 100
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (59)
health-services/geopode-adapter/README.md(1 hunks)health-services/geopode-adapter/pom.xml(1 hunks)health-services/geopode-adapter/src/main/java/digit/Main.java(1 hunks)health-services/geopode-adapter/src/main/java/digit/config/Configuration.java(1 hunks)health-services/geopode-adapter/src/main/java/digit/config/ServiceConstants.java(1 hunks)health-services/geopode-adapter/src/main/java/digit/kafka/Producer.java(1 hunks)health-services/geopode-adapter/src/main/java/digit/repository/ServiceRequestRepository.java(1 hunks)health-services/geopode-adapter/src/main/java/digit/service/GeopodeAdapterService.java(1 hunks)health-services/geopode-adapter/src/main/java/digit/util/ArcgisUtil.java(1 hunks)health-services/geopode-adapter/src/main/java/digit/util/BoundaryUtil.java(1 hunks)health-services/geopode-adapter/src/main/java/digit/util/MdmsUtil.java(1 hunks)health-services/geopode-adapter/src/main/java/digit/util/MdmsV2Util.java(1 hunks)health-services/geopode-adapter/src/main/java/digit/web/controllers/GeopodeApiController.java(1 hunks)health-services/geopode-adapter/src/main/java/digit/web/models/Arcgis/ArcgisRequest.java(1 hunks)health-services/geopode-adapter/src/main/java/digit/web/models/Arcgis/ArcgisResponse.java(1 hunks)health-services/geopode-adapter/src/main/java/digit/web/models/Arcgis/Attributes.java(1 hunks)health-services/geopode-adapter/src/main/java/digit/web/models/Arcgis/Feature.java(1 hunks)health-services/geopode-adapter/src/main/java/digit/web/models/Arcgis/Field.java(1 hunks)health-services/geopode-adapter/src/main/java/digit/web/models/Arcgis/Geometry.java(1 hunks)health-services/geopode-adapter/src/main/java/digit/web/models/Arcgis/GeometryProperties.java(1 hunks)health-services/geopode-adapter/src/main/java/digit/web/models/Arcgis/SpatialReference.java(1 hunks)health-services/geopode-adapter/src/main/java/digit/web/models/Arcgis/UniqueIdField.java(1 hunks)health-services/geopode-adapter/src/main/java/digit/web/models/GeopdeHierarchyLevel.java(1 hunks)health-services/geopode-adapter/src/main/java/digit/web/models/GeopodeBoundary.java(1 hunks)health-services/geopode-adapter/src/main/java/digit/web/models/GeopodeBoundaryRequest.java(1 hunks)health-services/geopode-adapter/src/main/java/digit/web/models/boundaryService/Boundary.java(1 hunks)health-services/geopode-adapter/src/main/java/digit/web/models/boundaryService/BoundaryHierarchy.java(1 hunks)health-services/geopode-adapter/src/main/java/digit/web/models/boundaryService/BoundaryHierarchyDefinitionResponse.java(1 hunks)health-services/geopode-adapter/src/main/java/digit/web/models/boundaryService/BoundaryHierarchyDefinitionSearchCriteria.java(1 hunks)health-services/geopode-adapter/src/main/java/digit/web/models/boundaryService/BoundaryHierarchyDefinitonSearchRequest.java(1 hunks)health-services/geopode-adapter/src/main/java/digit/web/models/boundaryService/BoundaryRelation.java(1 hunks)health-services/geopode-adapter/src/main/java/digit/web/models/boundaryService/BoundaryRelationshipDTO.java(1 hunks)health-services/geopode-adapter/src/main/java/digit/web/models/boundaryService/BoundaryRelationshipRequest.java(1 hunks)health-services/geopode-adapter/src/main/java/digit/web/models/boundaryService/BoundaryRelationshipRequestDTO.java(1 hunks)health-services/geopode-adapter/src/main/java/digit/web/models/boundaryService/BoundaryRelationshipResponse.java(1 hunks)health-services/geopode-adapter/src/main/java/digit/web/models/boundaryService/BoundaryRelationshipSearchCriteria.java(1 hunks)health-services/geopode-adapter/src/main/java/digit/web/models/boundaryService/BoundaryRequest.java(1 hunks)health-services/geopode-adapter/src/main/java/digit/web/models/boundaryService/BoundaryResponse.java(1 hunks)health-services/geopode-adapter/src/main/java/digit/web/models/boundaryService/BoundarySearchCriteria.java(1 hunks)health-services/geopode-adapter/src/main/java/digit/web/models/boundaryService/BoundarySearchResponse.java(1 hunks)health-services/geopode-adapter/src/main/java/digit/web/models/boundaryService/BoundaryType.java(1 hunks)health-services/geopode-adapter/src/main/java/digit/web/models/boundaryService/BoundaryTypeHierarchy.java(1 hunks)health-services/geopode-adapter/src/main/java/digit/web/models/boundaryService/BoundaryTypeHierarchyDefinition.java(1 hunks)health-services/geopode-adapter/src/main/java/digit/web/models/boundaryService/BoundaryTypeHierarchyRequest.java(1 hunks)health-services/geopode-adapter/src/main/java/digit/web/models/boundaryService/BoundaryTypeHierarchyResponse.java(1 hunks)health-services/geopode-adapter/src/main/java/digit/web/models/boundaryService/BoundaryTypeHierarchySearchCriteria.java(1 hunks)health-services/geopode-adapter/src/main/java/digit/web/models/boundaryService/BoundaryTypeHierarchySearchRequest.java(1 hunks)health-services/geopode-adapter/src/main/java/digit/web/models/boundaryService/EnrichedBoundary.java(1 hunks)health-services/geopode-adapter/src/main/java/digit/web/models/boundaryService/HierarchyRelation.java(1 hunks)health-services/geopode-adapter/src/main/java/digit/web/models/boundaryService/HierarchyType.java(1 hunks)health-services/geopode-adapter/src/main/java/digit/web/models/mdmsV2/Mdms.java(1 hunks)health-services/geopode-adapter/src/main/java/digit/web/models/mdmsV2/MdmsCriteriaReqV2.java(1 hunks)health-services/geopode-adapter/src/main/java/digit/web/models/mdmsV2/MdmsCriteriaV2.java(1 hunks)health-services/geopode-adapter/src/main/java/digit/web/models/mdmsV2/MdmsResponseV2.java(1 hunks)health-services/geopode-adapter/src/main/resources/Dockerfile(1 hunks)health-services/geopode-adapter/src/main/resources/application.properties(1 hunks)health-services/geopode-adapter/src/main/resources/start.sh(1 hunks)health-services/geopode-adapter/src/test/java/digit/TestConfiguration.java(1 hunks)health-services/geopode-adapter/src/test/java/digit/web/controllers/GeopodeApiControllerTest.java(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
health-services/geopode-adapter/src/main/java/digit/config/ServiceConstants.java (1)
Learnt from: Saloni-eGov
PR: egovernments/health-campaign-services#906
File: health-services/plan-service/src/main/java/digit/util/CampaignUtil.java:1-13
Timestamp: 2024-09-26T10:49:23.404Z
Learning: When importing constants from 'digit.config.ServiceConstants', it's acceptable to use static wildcard imports since all constants are static.
🧬 Code Graph Analysis (8)
health-services/geopode-adapter/src/main/java/digit/web/models/Arcgis/Feature.java (3)
health-services/geopode-adapter/src/main/java/digit/web/models/Arcgis/Attributes.java (1)
Data(10-19)health-services/geopode-adapter/src/main/java/digit/web/models/Arcgis/ArcgisResponse.java (1)
Data(13-48)health-services/geopode-adapter/src/main/java/digit/web/models/Arcgis/Geometry.java (1)
Data(12-21)
health-services/geopode-adapter/src/main/java/digit/web/models/boundaryService/BoundaryTypeHierarchy.java (3)
health-services/geopode-adapter/src/main/java/digit/web/models/boundaryService/BoundaryType.java (1)
Validated(14-36)health-services/geopode-adapter/src/main/java/digit/web/models/boundaryService/BoundaryTypeHierarchyDefinition.java (1)
Validated(18-45)health-services/geopode-adapter/src/main/java/digit/web/models/boundaryService/BoundaryTypeHierarchyRequest.java (1)
Validated(15-31)
health-services/geopode-adapter/src/main/java/digit/web/models/Arcgis/UniqueIdField.java (1)
health-services/geopode-adapter/src/main/java/digit/web/models/Arcgis/ArcgisResponse.java (1)
Data(13-48)
health-services/geopode-adapter/src/main/java/digit/web/models/boundaryService/BoundaryTypeHierarchyRequest.java (1)
health-services/geopode-adapter/src/main/java/digit/web/models/boundaryService/BoundaryTypeHierarchyDefinition.java (1)
Validated(18-45)
health-services/geopode-adapter/src/main/java/digit/web/models/Arcgis/ArcgisRequest.java (9)
health-services/geopode-adapter/src/main/java/digit/web/models/Arcgis/Attributes.java (1)
Data(10-19)health-services/geopode-adapter/src/main/java/digit/web/models/Arcgis/ArcgisResponse.java (1)
Data(13-48)health-services/geopode-adapter/src/main/java/digit/web/models/Arcgis/Field.java (1)
Data(10-37)health-services/geopode-adapter/src/main/java/digit/web/models/Arcgis/Geometry.java (1)
Data(12-21)health-services/geopode-adapter/src/main/java/digit/web/models/Arcgis/Feature.java (1)
Data(11-25)health-services/geopode-adapter/src/main/java/digit/web/models/Arcgis/SpatialReference.java (1)
Data(10-22)health-services/geopode-adapter/src/main/java/digit/web/models/Arcgis/GeometryProperties.java (1)
Data(10-25)health-services/geopode-adapter/src/main/java/digit/web/models/Arcgis/UniqueIdField.java (1)
Data(10-22)health-services/geopode-adapter/src/main/java/digit/web/models/GeopodeBoundaryRequest.java (1)
Validated(15-27)
health-services/geopode-adapter/src/main/java/digit/web/models/boundaryService/BoundaryResponse.java (1)
health-services/geopode-adapter/src/main/java/digit/web/models/boundaryService/Boundary.java (1)
Validated(17-44)
health-services/geopode-adapter/src/main/java/digit/web/models/boundaryService/BoundarySearchCriteria.java (1)
health-services/geopode-adapter/src/main/java/digit/web/models/boundaryService/BoundaryRelationshipSearchCriteria.java (1)
Validated(16-61)
health-services/geopode-adapter/src/test/java/digit/web/controllers/GeopodeApiControllerTest.java (1)
health-services/geopode-adapter/src/main/java/digit/Main.java (1)
Import(9-17)
🪛 Shellcheck (0.10.0)
health-services/geopode-adapter/src/main/resources/start.sh
[warning] 3-3: In POSIX sh, [[ ]] is undefined.
(SC3010)
[info] 11-11: Double quote to prevent globbing and word splitting.
(SC2086)
[info] 11-11: Double quote to prevent globbing and word splitting.
(SC2086)
[info] 11-11: Double quote to prevent globbing and word splitting.
(SC2086)
🪛 LanguageTool
health-services/geopode-adapter/README.md
[misspelling] ~13-~13: Use “a” instead of ‘an’ if the following word doesn’t start with a vowel sound, e.g. ‘a sentence’, ‘a university’.
Context: ...gfox/springfox) Start your server as an simple java application You can view...
(EN_A_VS_AN)
[uncategorized] ~18-~18: You might be missing the article “the” here.
Context: ...g to http://localhost:8080/ Change default port value in application.properties
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
🪛 markdownlint-cli2 (0.17.2)
health-services/geopode-adapter/README.md
3-3: Trailing spaces
Expected: 0 or 2; Actual: 1
(MD009, no-trailing-spaces)
5-5: Multiple consecutive blank lines
Expected: 1; Actual: 2
(MD012, no-multiple-blanks)
6-6: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
16-16: Bare URL used
null
(MD034, no-bare-urls)
18-18: Files should end with a single newline character
null
(MD047, single-trailing-newline)
🪛 Checkov (3.2.334)
health-services/geopode-adapter/src/main/resources/Dockerfile
[LOW] 1-20: Ensure that HEALTHCHECK instructions have been added to container images
(CKV_DOCKER_2)
[LOW] 1-20: Ensure that a user for the container has been created
(CKV_DOCKER_3)
🪛 Hadolint (2.12.0)
health-services/geopode-adapter/src/main/resources/Dockerfile
[warning] 6-6: Pin versions in apk add. Instead of apk add <package> use apk add <package>=<version>
(DL3018)
🔇 Additional comments (36)
health-services/geopode-adapter/src/main/java/digit/config/ServiceConstants.java (1)
10-31: Well-organized constants structure.The error messages and common constants are well-defined and follow consistent naming conventions. The hierarchical organization makes them easy to locate and use.
health-services/geopode-adapter/src/main/java/digit/web/models/Arcgis/SpatialReference.java (1)
1-22: Well-implemented model class following best practices.The
SpatialReferenceclass is properly structured with:
- Appropriate Lombok annotations for boilerplate reduction
- Correct Jackson annotations for JSON mapping
- Proper field types for WKID (Well-Known Identifier) values
@JsonIgnoreProperties(ignoreUnknown = true)for robust deserializationThis follows standard patterns for ArcGIS spatial reference data models.
health-services/geopode-adapter/src/main/resources/start.sh (1)
3-10: Well-structured startup logic with proper configuration handling.The script properly handles:
- Default JVM memory settings with fallback values
- Conditional remote debugging configuration
- Environment variable-based configuration
The logic for setting up debugging and memory options is sound.
🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 3-3: In POSIX sh, [[ ]] is undefined.
(SC3010)
health-services/geopode-adapter/src/main/java/digit/kafka/Producer.java (1)
8-19: LGTM! Well-implemented Kafka producer service.The implementation follows good practices:
- Clear separation of concerns with a dedicated producer service
- Proper use of CustomKafkaTemplate for tracing integration
- Helpful comment about KafkaTemplate alternative
- Simple and focused API with the push method
health-services/geopode-adapter/src/main/java/digit/web/models/Arcgis/Geometry.java (1)
12-21: LGTM! Well-designed data model for ArcGIS geometry.The implementation follows good practices for data model classes:
- Proper use of Lombok annotations to reduce boilerplate
- Jackson annotations for robust JSON serialization/deserialization
- @JsonIgnoreProperties ensures compatibility with evolving APIs
- Appropriate data structure (3-level list) for representing ring coordinates
- Builder pattern support for flexible object creation
health-services/geopode-adapter/src/test/java/digit/TestConfiguration.java (1)
9-16: LGTM! Clean test configuration setup.The test configuration follows Spring Boot testing best practices by providing a mocked KafkaTemplate bean. The @SuppressWarnings annotation is appropriately used to suppress the unchecked cast warning when mocking generic types.
health-services/geopode-adapter/src/main/java/digit/web/models/Arcgis/Feature.java (1)
11-25: LGTM! Well-structured ArcGIS feature model.The
Featureclass is well-designed with:
- Proper use of
@Validannotations for nested object validation- Clear and descriptive field names
- Helpful inline comments explaining the purpose of each field
- Good integration with the broader ArcGIS model hierarchy
health-services/geopode-adapter/src/main/java/digit/web/models/GeopodeBoundaryRequest.java (1)
1-27: LGTM! Well-structured request DTO.The class follows standard DTO patterns with proper use of:
- Lombok annotations for boilerplate code reduction
- Jackson annotations for JSON serialization
- Validation annotations for request validation
- Clean and readable structure
health-services/geopode-adapter/src/main/java/digit/web/models/mdmsV2/MdmsResponseV2.java (1)
14-28: LGTM! Well-structured response DTO.The class properly uses Lombok annotations and Jackson properties for a clean response model implementation.
health-services/geopode-adapter/src/main/java/digit/web/models/Arcgis/GeometryProperties.java (1)
1-25: LGTM! Clean and well-structured DTO.The class follows excellent DTO patterns with:
- Proper Lombok annotations for code generation
- Appropriate Jackson annotations for JSON handling
@JsonIgnoreProperties(ignoreUnknown = true)for flexible deserialization- Descriptive field names following camelCase convention
health-services/geopode-adapter/src/main/java/digit/web/models/boundaryService/HierarchyType.java (1)
16-36: LGTM! Proper DTO implementation with validation.The class correctly uses:
- Lombok annotations for boilerplate code generation
- Validation annotations including
@NotNullon the requiredcodefield- Appropriate JSON property mappings
- Clean field structure for hierarchy type representation
health-services/geopode-adapter/src/main/java/digit/web/models/boundaryService/BoundaryRelationshipRequestDTO.java (1)
18-26:❓ Verification inconclusive
Clarify the design intent for having both single and list fields.
Having both a single
BoundaryRelationshipDTOand a list ofBoundaryRelationshipDTOin the same request object creates ambiguity. Consider whether both are needed or if they serve different purposes.Please clarify the use case:
- Is this intended for batch operations where you can send either a single item OR a list?
- Are both fields used simultaneously for different purposes?
- Should this be split into separate request classes?
This design pattern is not seen in other similar request classes in the codebase.
🏁 Script executed:
#!/bin/bash # List all RequestDTO.java files and their @JsonProperty fields find health-services/geopode-adapter/src/main/java/digit/web/models -type f -name "*RequestDTO.java" | while read file; do echo "== $file ==" grep -n "@JsonProperty" "$file" || echo " (no @JsonProperty annotations found)" echo doneLength of output: 654
Clarify dual usage of
BoundaryRelationshipDTOinBoundaryRelationshipRequestDTOThe presence of both a single
boundaryRelationshipDTOand a listboundaryRelationshipDTOListwithin the same request class introduces ambiguity. Other request DTOs in this module typically accept either one or the other, but not both.File:
health-services/geopode-adapter/src/main/java/digit/web/models/boundaryService/BoundaryRelationshipRequestDTO.java (lines 18–26)@JsonProperty("BoundaryRelationship") private BoundaryRelationshipDTO boundaryRelationshipDTO; @JsonProperty("RequestInfo") private RequestInfo requestInfo; @JsonProperty("BoundaryRelationshipList") private List<BoundaryRelationshipDTO> boundaryRelationshipDTOList;Please clarify the intended design:
- Should clients send either a single item or a list in this endpoint?
- Are both fields ever populated simultaneously for distinct purposes?
- Would it be clearer to split this into separate request classes (e.g., one for single vs. one for batch operations)?
health-services/geopode-adapter/src/main/java/digit/web/models/boundaryService/BoundaryResponse.java (1)
35-41: LGTM! Well-implemented helper method.The
addBoundaryItemmethod correctly handles null initialization and follows builder pattern conventions by returningthisfor method chaining.health-services/geopode-adapter/src/main/java/digit/web/models/boundaryService/BoundaryType.java (1)
14-36: LGTM! Well-structured model class.The class follows good practices with:
- Appropriate validation (
@NotNullon thecodefield)- Proper Lombok annotations for boilerplate reduction
- Standard Jackson annotations for JSON serialization
- Clean, minimal design suitable for a data model
health-services/geopode-adapter/src/main/java/digit/web/models/boundaryService/BoundaryTypeHierarchy.java (1)
13-30: LGTM! Appropriate design for hierarchy representation.The class correctly models boundary type hierarchies using String references, which is suitable for JSON serialization and avoids circular dependency issues that could occur with object references.
health-services/geopode-adapter/src/main/java/digit/web/models/Arcgis/UniqueIdField.java (1)
10-22: LGTM! Well-designed ArcGIS model class.The class demonstrates good practices:
- Uses
@JsonIgnoreProperties(ignoreUnknown = true)for robustness with external API responses- Primitive
booleantype forisSystemMaintainedis appropriate if this field always has a value in ArcGIS responses- Clean, focused design suitable for its purpose as part of the ArcGIS integration
health-services/geopode-adapter/src/main/java/digit/web/models/boundaryService/BoundaryRelationshipResponse.java (1)
23-32: LGTM: Well-structured response model.The response structure follows standard patterns with proper validation annotations. The use of
@Validon theResponseInfoandList<BoundaryRelation>fields is appropriate for nested object validation.health-services/geopode-adapter/src/main/java/digit/web/models/boundaryService/BoundaryRelationshipDTO.java (1)
17-46: LGTM: Well-designed DTO with proper validation.This DTO demonstrates good design practices:
- Appropriate use of
@NotNullvalidation on required business fields- Inclusion of audit details for traceability
- The
ancestralMaterializedPathfield indicates proper support for hierarchical boundary structures- Clean field naming and structure
health-services/geopode-adapter/src/main/java/digit/web/models/boundaryService/BoundaryTypeHierarchyRequest.java (1)
21-30: LGTM: Standard request wrapper pattern.This class follows the established eGov platform pattern for request DTOs with proper validation annotations on both the
RequestInfoand business object fields.health-services/geopode-adapter/src/main/java/digit/web/models/boundaryService/BoundaryTypeHierarchySearchCriteria.java (1)
21-38: LGTM! Well-structured search criteria class.The implementation follows standard patterns with:
- Proper validation on required fields (
tenantId)- Optional
hierarchyTypefor flexible searching- Standard pagination support with
offsetandlimit- Appropriate size constraints and JSON property mapping
health-services/geopode-adapter/src/main/java/digit/web/models/boundaryService/BoundaryTypeHierarchyResponse.java (1)
23-36: LGTM! Properly structured response class.The implementation follows best practices with:
- Complete annotation set including
@Validated,@Generated, and@Valid- Proper validation on complex objects
- Standard response structure with
ResponseInfo,totalCount, and data list- Consistent JSON property mapping
health-services/geopode-adapter/src/main/java/digit/web/models/boundaryService/BoundaryTypeHierarchySearchRequest.java (1)
23-29: Good use of validation annotations.The use of
@Validannotations on the fields ensures proper validation cascading, which is a good practice for request DTOs.health-services/geopode-adapter/src/main/java/digit/web/models/boundaryService/BoundaryRelationshipRequest.java (1)
21-32: Well-structured DTO implementation.The class follows proper DTO patterns with appropriate annotations, validation, and field types. The implementation is clean and consistent with other boundary service models.
health-services/geopode-adapter/src/main/java/digit/web/models/boundaryService/EnrichedBoundary.java (1)
40-41:❌ Incorrect review comment
Consider implications of @JsonIgnore on parent field.
The
parentfield is excluded from JSON serialization, which means:
- When deserializing JSON, parent relationships won't be populated
- Bidirectional navigation between parent-child boundaries won't work properly
If parent-child relationships are important for the domain logic, consider either including the parent in JSON or establishing parent relationships programmatically after deserialization.
🏁 Script executed:
#!/bin/bash # Description: Check how EnrichedBoundary is used in the codebase to understand if parent field exclusion is problematic # Search for usage of EnrichedBoundary parent field rg -A 3 -B 3 "\.parent" --type java # Search for parent-child relationship usage rg -A 5 "EnrichedBoundary.*parent\|parent.*EnrichedBoundary" --type javaLength of output: 7968
No usages of
parentin EnrichedBoundary – safe to ignore@JsonIgnoreA repository‐wide search for any access to
EnrichedBoundary.parent(or its getter) returned no references. Since no code reads or writes that field, excluding it from JSON has no effect on existing logic.• No
.parentorgetParent()calls onEnrichedBoundaryfound
• No serialization or deserialization paths depend on that fieldYou can safely remove this comment.
Likely an incorrect or invalid review comment.
health-services/geopode-adapter/src/main/java/digit/web/models/boundaryService/BoundarySearchCriteria.java (1)
22-25: LGTM on the codes field validation.The validation constraints are appropriate - ensuring the list is not null and contains at least one element.
health-services/geopode-adapter/src/main/java/digit/web/models/Arcgis/ArcgisResponse.java (2)
13-18: Excellent use of annotations for external API integration.The combination of
@JsonIgnoreProperties(ignoreUnknown = true)and proper Lombok annotations makes this a robust response model for external ArcGIS services.
20-47: Good use of @Valid annotations for nested validation.All complex nested objects properly use
@Validannotation, ensuring that validation cascades through the object hierarchy when this response is processed.health-services/geopode-adapter/src/main/java/digit/web/models/boundaryService/BoundaryRelation.java (3)
43-44: Good design: parent field appropriately optional.Correctly allowing null parent for root boundary nodes.
46-47: Excellent inclusion of audit details.Including
AuditDetailsfollows best practices for entity tracking and audit trails.
49-50: Well-designed internal field for hierarchy operations.The
@JsonIgnoreancestral materialized path field with empty string default is a good design choice for internal hierarchy traversal without exposing it in the API.health-services/geopode-adapter/src/main/java/digit/web/models/GeopdeHierarchyLevel.java (2)
10-15: Clean and well-structured model class.Good use of Lombok annotations and
@JsonIgnorePropertiesfor flexible external data parsing.
17-27: Proper JSON property mapping with good field naming.The mapping from snake_case JSON properties to camelCase Java fields follows conventions. Using primitive
intfor level is appropriate since hierarchy levels are typically non-null.health-services/geopode-adapter/src/main/java/digit/web/models/mdmsV2/MdmsCriteriaReqV2.java (1)
18-26:❌ Incorrect review comment
Verify MdmsCriteriaV2 import and usage.
The class references
MdmsCriteriaV2but the import is not visible in this file. Ensure that this class exists and is properly imported.
🏁 Script executed:
#!/bin/bash # Description: Verify MdmsCriteriaV2 class exists and is properly used # Expected: Find the MdmsCriteriaV2 class definition and its usage # Search for MdmsCriteriaV2 class definition echo "Searching for MdmsCriteriaV2 class definition:" fd -e java | xargs grep -l "class MdmsCriteriaV2" # Search for imports of MdmsCriteriaV2 echo "Searching for MdmsCriteriaV2 imports:" rg "import.*MdmsCriteriaV2" -A 2 # Check usage patterns echo "Checking usage patterns:" rg "MdmsCriteriaV2" -A 2 -B 2Length of output: 8603
No import needed for MdmsCriteriaV2 in this file
MdmsCriteriaReqV2 and MdmsCriteriaV2 both live indigit.web.models.mdmsV2, so an explicit import isn’t required. TheMdmsCriteriaV2class is present atsrc/main/java/digit/web/models/mdmsV2/MdmsCriteriaV2.javaand is correctly referenced.Likely an incorrect or invalid review comment.
health-services/geopode-adapter/src/main/java/digit/repository/ServiceRequestRepository.java (1)
15-15:✅ Verification successful
Verify ServiceConstants import and usage.
The code uses constants from
ServiceConstantsbut ensure they are properly defined for the error scenarios used.
🏁 Script executed:
#!/bin/bash # Description: Verify ServiceConstants contains the required error constants # Expected: Find EXTERNAL_SERVICE_EXCEPTION and SEARCHER_SERVICE_EXCEPTION definitions echo "Searching for ServiceConstants class and error constants:" rg "EXTERNAL_SERVICE_EXCEPTION|SEARCHER_SERVICE_EXCEPTION" -A 2 -B 2 echo "Checking ServiceConstants class definition:" fd -name "ServiceConstants.java" | xargs catLength of output: 11011
ServiceConstants import and usage verified
The constants
EXTERNAL_SERVICE_EXCEPTIONandSEARCHER_SERVICE_EXCEPTIONare present inhealth-services/geopode-adapter/src/main/java/digit/config/ServiceConstants.java, so theimport static digit.config.ServiceConstants.*;is valid and requires no changes.health-services/geopode-adapter/src/main/java/digit/util/ArcgisUtil.java (1)
99-99: 🧹 Nitpick (assertive)Remove redundant parentheses.
Apply this diff:
- .map(mdms -> mdms.getData()) + .map(mdms -> mdms.getData())Likely an incorrect or invalid review comment.
health-services/geopode-adapter/src/main/java/digit/web/models/boundaryService/BoundaryRelationshipSearchCriteria.java (1)
37-39: Verify null handling for the codes field.The
codesfield is initialized tonull. Ensure that all code accessing this field performs proper null checks to avoid NullPointerException.Consider initializing with an empty list if the field is frequently accessed:
@JsonProperty("codes") - private List<String> codes = null; + @Builder.Default + private List<String> codes = new ArrayList<>();
health-services/geopode-adapter/src/main/java/digit/config/ServiceConstants.java
Show resolved
Hide resolved
health-services/geopode-adapter/src/main/java/digit/config/ServiceConstants.java
Outdated
Show resolved
Hide resolved
health-services/geopode-adapter/src/main/java/digit/config/ServiceConstants.java
Show resolved
Hide resolved
...apter/src/main/java/digit/web/models/boundaryService/BoundaryTypeHierarchySearchRequest.java
Show resolved
Hide resolved
health-services/geopode-adapter/src/main/resources/application.properties
Show resolved
Hide resolved
health-services/geopode-adapter/src/main/resources/application.properties
Show resolved
Hide resolved
health-services/geopode-adapter/src/main/resources/application.properties
Show resolved
Hide resolved
health-services/geopode-adapter/src/main/resources/application.properties
Show resolved
Hide resolved
health-services/geopode-adapter/src/main/java/digit/config/Configuration.java
Outdated
Show resolved
Hide resolved
| public static final String FORMAT_VALUE="json"; | ||
| public static final String ERROR_FETCHING_FROM_BOUNDARY="Error Fetching Data from boundary"; | ||
| public static final String ROOT_BOUNDARY_ALREADY_EXISTS="Root Boundary already created"; | ||
| public static final String RESPONSE_FROM_GEOPODE_API = "[\n" + |
There was a problem hiding this comment.
this will be fetched from boundary-service heirarchy
There was a problem hiding this comment.
When call is being made to boundary, service and error comes, I use these constants in the catch
There was a problem hiding this comment.
why is error coming?
health-services/geopode-adapter/src/main/java/digit/service/GeopodeAdapterService.java
Show resolved
Hide resolved
health-services/geopode-adapter/src/main/java/digit/service/GeopodeAdapterService.java
Outdated
Show resolved
Hide resolved
health-services/geopode-adapter/src/main/java/digit/service/GeopodeAdapterService.java
Show resolved
Hide resolved
health-services/geopode-adapter/src/main/java/digit/service/GeopodeAdapterService.java
Show resolved
Hide resolved
health-services/geopode-adapter/src/main/java/digit/config/Configuration.java
Outdated
Show resolved
Hide resolved
health-services/geopode-adapter/src/main/java/digit/repository/ServiceRequestRepository.java
Show resolved
Hide resolved
health-services/geopode-adapter/src/main/java/digit/repository/ServiceRequestRepository.java
Outdated
Show resolved
Hide resolved
health-services/geopode-adapter/src/main/java/digit/repository/ServiceRequestRepository.java
Outdated
Show resolved
Hide resolved
health-services/geopode-adapter/src/main/java/digit/util/ArcgisUtil.java
Outdated
Show resolved
Hide resolved
health-services/geopode-adapter/src/main/java/digit/util/ArcgisUtil.java
Outdated
Show resolved
Hide resolved
health-services/geopode-adapter/src/main/java/digit/util/ArcgisUtil.java
Outdated
Show resolved
Hide resolved
health-services/geopode-adapter/src/main/java/digit/util/ArcgisUtil.java
Outdated
Show resolved
Hide resolved
health-services/geopode-adapter/src/main/java/digit/util/ArcgisUtil.java
Outdated
Show resolved
Hide resolved
| * @return | ||
| */ | ||
| private String extractCountryNameFromMdms(MdmsResponseV2 mdmsResponse, String targetIsoCode) { | ||
| return mdmsResponse.getMdms().stream() |
There was a problem hiding this comment.
see if you can make this mode readable
3 participants
HCMPRE-2701: Creating root boundary node
Summary by CodeRabbit
New Features
Documentation
Tests