HCMPRE-1576,1577,1578: Removed readme sheet, localised estimation sheet and added facility name #1314
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 WalkthroughThe pull request introduces several enhancements to the health services resource generator module. New utility classes and modifications to existing classes focus on improving Excel file processing, localization, and data enrichment. The changes include adding a new Changes
Sequence DiagramsequenceDiagram
participant EP as ExcelParser
participant OEG as OutputEstimationGenerationUtil
participant ESU as ExcelStylingUtil
participant PU as ParsingUtil
EP->>OEG: processOutputFile
OEG->>PU: isSheetAllowedToProcess
OEG->>ESU: styleCell
OEG-->>EP: Processed Workbook
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: 11
🔭 Outside diff range comments (1)
health-services/resource-generator/src/main/java/org/egov/processor/service/ExcelParser.java (1)
Line range hint
203-227: Sheet-by-Sheet Processing
processSheets:
- Relies on
parsingUtil.isSheetAllowedToProcess.- Depending on the plan status, either enriches the sheet with census records, processes rows for calculations, or enriches plan estimates.
- The logic is open for extension. Consider adding further validation or logging when skipping sheets.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (6)
health-services/resource-generator/src/main/java/org/egov/processor/config/ServiceConstants.java(2 hunks)health-services/resource-generator/src/main/java/org/egov/processor/service/ExcelParser.java(8 hunks)health-services/resource-generator/src/main/java/org/egov/processor/util/EnrichmentUtil.java(4 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/OutputEstimationGenerationUtil.java(1 hunks)health-services/resource-generator/src/main/java/org/egov/processor/util/ParsingUtil.java(5 hunks)
🔇 Additional comments (22)
health-services/resource-generator/src/main/java/org/egov/processor/util/ExcelStylingUtil.java (2)
38-41: Locking cells requires sheet protection.
SettingcellStyle.setLocked(true)only takes effect if the sheet is also protected. Otherwise, the locked property will not restrict edits. If the intent is to actually prevent edits, ensure the sheet is protected.
75-77: 🧹 Nitpick (assertive)Confirm the necessity of runtime color transformation.
Increasing RGB components by 10% can alter brand or design guidelines. Verify that consuming modules and design teams are aware and approve of this runtime color shift.health-services/resource-generator/src/main/java/org/egov/processor/util/OutputEstimationGenerationUtil.java (6)
1-9: Well-structured Class DeclarationThe new class
OutputEstimationGenerationUtilis neatly introduced with relevant imports and a clear@Componentannotation. This promotes good Spring-based design, ensuring the utility is injectable where required.
32-37: Constructor DependenciesThe constructor correctly injects the required utilities (
LocaleUtil,ParsingUtil,EnrichmentUtil,ExcelStylingUtil). This is a good pattern for maintaining testability and clear dependency management.
39-59: Localizing Headers in SheetsThe
processOutputFilemethod:
- Removes sheets not permitted by
parsingUtil.isSheetAllowedToProcess.- Builds a localization map from the
LocaleResponse.- Iterates through remaining sheets to localize header cells.
These steps show a cohesive approach to eliminating irrelevant sheets and localizing headers. The logic is clear and modular.
124-148: Boundary-Facility Mapping
getBoundaryCodeToFacilityMapefficiently:
- Gathers boundary codes from all allowed sheets.
- Fetches census data for these boundary codes.
- Maps the relevant census record field to
FACILITY_NAME.This approach cleanly encapsulates the logic for building the boundary-to-facility relationship. The usage of streams for collecting to a map is concise and effective.
150-178: Populating Facility Column by Boundary Code
addFacilityNameToSheetcreates and populates the new facility column for each row:
- Retrieves the boundary code from the row.
- Looks up the facility in the
boundaryCodeToFacility.- Writes the facility name to the newly created column.
Ensure that you handle rows where the boundary code cell is blank or typed incorrectly, to avoid potential
NullPointerExceptions (line 169). For example, add a null/empty check before callinggetStringCellValue().
180-194: Creating the New Column
createAssignedFacilityColumnappends the new facility column to the header row:
- Sets the column type to
STRING.- Styles the header cell.
- Returns the column index for subsequent population.
This is a neat and straightforward method, nicely separated from the logic that populates the column’s cells. Good job keeping the styling concerns isolated in
excelStylingUtil.health-services/resource-generator/src/main/java/org/egov/processor/util/ParsingUtil.java (6)
10-11: Importing Locale ClassesThe addition of
Localeimports aligns with the new locale-based checks in the utility. These imports ensure the class has the references needed to integrate localization data seamlessly.
20-20: Added BigDecimal SupportThe import of
java.math.BigDecimalsuggests that numeric conversions in newly introduced methods now require high-precision arithmetic. This is a good approach to avoid floating-point imprecision.
28-28: New MDMS Field Constants
import static org.egov.processor.config.ServiceConstants.*;Pulling in constants from
ServiceConstantshelps maintain consistent naming and reduce duplication for error codes, messages, and property field names across the codebase.
40-43: Constructor ExpansionAdding
MdmsUtil mdmsUtilandObjectMapper objectMapperextends the parsing capabilities, supporting dynamic MDMS data retrieval and JSON processing. This is a positive step toward scalability, as it cleanly injects additional dependencies.
324-324: Removing Static fromisRowEmptyMaking
isRowEmptynon-static enables the method to leverage instance-level dependencies if needed. However, ensure no external usage breaks from the signature change if the method was previously accessed in a static manner.
402-422: Sheet Eligibility Check for Processing
isSheetAllowedToProcess:
- Fetches MDMS data for common constants.
- Loops over locale messages and checks if the sheet name matches certain read-me or boundary data criteria.
- Returns
falseto skip disallowed sheets.This method centralizes “which sheets are valid” logic nicely, and it’s used consistently in other classes. Good approach!
health-services/resource-generator/src/main/java/org/egov/processor/service/ExcelParser.java (5)
63-64: Introducing OutputEstimationGenerationUtilThe new
outputEstimationGenerationUtildependency indicates a design choice to shift estimation logic out ofExcelParser. This fosters a single-responsibility approach and keeps the parser’s scope narrower.
Line range hint
67-81: Constructor UpdateExpanding the constructor to accept multiple new parameters, including
OutputEstimationGenerationUtil, ensures that the parser can delegate tasks like generating final output files and localizing them. This is consistent with good constructor-based dependency injection.
130-130: Uploading and Integrating CampaignIn
processExcelFile, the method reference foruploadFileAndIntegrateCampaignis extended, reflecting a new workflow for generating final output files. This ensures a more unified approach to file and campaign processing.
Line range hint
155-180: Conditions for File Upload and IntegrationInside
uploadFileAndIntegrateCampaign:
- If the plan status requires “PlanEstimates,” the file is uploaded and the plan updated.
- If the plan status is “UpdatePlanEstimatesIntoOutputFile” (line 165), the output file is integrated into the project factory, processed for facility assignment (lines 170-173), re-uploaded, and updated in the plan configuration.
This flow is thorough and ensures consistent data across the system. The transitions between statuses are well-defined and the logic is easy to follow.
227-227: Enriching with Approved Plan EstimatesNote that you’ve consistently used
enrichmentUtilfor different statuses. This is a nice example of the open/closed principle—logic can expand without rewriting the parser core. No major issues spotted here.health-services/resource-generator/src/main/java/org/egov/processor/config/ServiceConstants.java (2)
42-44: Key-Not-Present Constants
PROVIDED_KEY_IS_NOT_PRESENT_IN_JSON_OBJECT_*constants clearly communicate the error scenario for missing JSON keys. This enhances maintainability and fosters consistent error handling in the code.
122-123: Facility Name Constants
FACILITY_NAMEandHCM_MICROPLAN_SERVING_FACILITYare straightforward. They help avoid scattered string literals across the code. Great for clarity and avoiding typos.health-services/resource-generator/src/main/java/org/egov/processor/util/EnrichmentUtil.java (1)
226-229: Plan Records for EnrichmentQuerying plan records via
getPlanRecordsForEnrichmentand storing them in a map for quick lookups is consistent with how census records are handled. This symmetry improves maintainability.
health-services/resource-generator/src/main/java/org/egov/processor/util/ExcelStylingUtil.java
Show resolved
Hide resolved
health-services/resource-generator/src/main/java/org/egov/processor/util/ExcelStylingUtil.java
Outdated
Show resolved
Hide resolved
health-services/resource-generator/src/main/java/org/egov/processor/util/ExcelStylingUtil.java
Outdated
Show resolved
Hide resolved
...resource-generator/src/main/java/org/egov/processor/util/OutputEstimationGenerationUtil.java
Outdated
Show resolved
Hide resolved
...resource-generator/src/main/java/org/egov/processor/util/OutputEstimationGenerationUtil.java
Outdated
Show resolved
Hide resolved
health-services/resource-generator/src/main/java/org/egov/processor/service/ExcelParser.java
Outdated
Show resolved
Hide resolved
...th-services/resource-generator/src/main/java/org/egov/processor/config/ServiceConstants.java
Show resolved
Hide resolved
health-services/resource-generator/src/main/java/org/egov/processor/util/EnrichmentUtil.java
Outdated
Show resolved
Hide resolved
health-services/resource-generator/src/main/java/org/egov/processor/util/EnrichmentUtil.java
Outdated
Show resolved
Hide resolved
health-services/resource-generator/src/main/java/org/egov/processor/util/EnrichmentUtil.java
Outdated
Show resolved
Hide resolved
…health-campaign-services into download-estimation-1578
…health-campaign-services into download-estimation-1578
…ownload-estimation-1578
…ownload-estimation-1578 # Conflicts: # health-services/resource-generator/src/main/java/org/egov/processor/util/OutputEstimationGenerationUtil.java
…ownload-estimation-1578
Summary by CodeRabbit
New Features
Chores
Refactor