Develop #801
Develop #801
Conversation
PFM-5638: Added changes for penalty as well as UI and PDF issues res…
WalkthroughThe recent updates introduce penalty calculation and demand management features across multiple modules in the project. Key additions include new classes and methods for handling aggregated demand details, penalty configuration, and demand updates. A new Penalty Scheduler application is also added to automate penalty application processes. Changes
Sequence Diagram(s) (Beta)sequenceDiagram
participant User
participant DemandController
participant DemandService
participant DemandRepository
participant PenaltyScheduler
User->>+DemandController: POST /getAggregatedDemandDetails
DemandController->>+DemandService: getAggregatedDemandDetails
DemandService->>+DemandRepository: fetchDemandDetails
DemandRepository-->>-DemandService: Demand Details
DemandService-->>-DemandController: Aggregated Demand Details
DemandController-->>-User: Aggregated Demand Details
PenaltyScheduler->>+DemandService: addPenalty
DemandService->>+DemandRepository: getDemandsToAddPenalty
DemandRepository-->>-DemandService: Demands List
DemandService->>+DemandRepository: updateDemandsWithPenalty
DemandRepository-->>-DemandService: Updated Demands
DemandService-->>-PenaltyScheduler: Penalty Added Confirmation
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 as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
Outside diff range and nitpick comments (3)
utilities/Penalty/pom.xml (2)
19-21: Ensure thatspring-boot-starter-data-jpais necessary for your project as it adds significant overhead. If JPA is not used, consider removing this dependency.
32-32: Markinglombokas optional might lead to issues in environments where Lombok is not configured. Ensure all developers and the build environment include Lombok support.municipal-services/ws-calculator/src/main/java/org/egov/wscalculation/consumer/DemandGenerationConsumer.java (1)
36-36: Import optimization needed.Consider using specific imports instead of wildcard imports for better clarity and to avoid unnecessary loading of unused classes.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (25)
- business-services/billing-service/src/main/java/org/egov/demand/model/AggregatedDemandDetailResponse.java (1 hunks)
- business-services/billing-service/src/main/java/org/egov/demand/service/DemandService.java (4 hunks)
- business-services/billing-service/src/main/java/org/egov/demand/util/Constants.java (1 hunks)
- business-services/billing-service/src/main/java/org/egov/demand/web/controller/DemandController.java (2 hunks)
- municipal-services/ws-calculator/src/main/java/org/egov/wscalculation/config/WSCalculationConfiguration.java (1 hunks)
- municipal-services/ws-calculator/src/main/java/org/egov/wscalculation/constants/WSCalculationConstant.java (1 hunks)
- municipal-services/ws-calculator/src/main/java/org/egov/wscalculation/consumer/DemandGenerationConsumer.java (2 hunks)
- municipal-services/ws-calculator/src/main/java/org/egov/wscalculation/repository/DemandRepository.java (3 hunks)
- municipal-services/ws-calculator/src/main/java/org/egov/wscalculation/repository/builder/DemandQueryBuilder.java (1 hunks)
- municipal-services/ws-calculator/src/main/java/org/egov/wscalculation/service/DemandService.java (3 hunks)
- municipal-services/ws-calculator/src/main/java/org/egov/wscalculation/util/CalculatorUtil.java (1 hunks)
- municipal-services/ws-calculator/src/main/java/org/egov/wscalculation/web/controller/CalculatorController.java (2 hunks)
- municipal-services/ws-calculator/src/main/java/org/egov/wscalculation/web/models/AddPenaltyCriteria.java (1 hunks)
- municipal-services/ws-calculator/src/main/java/org/egov/wscalculation/web/models/PenaltyRequest.java (1 hunks)
- municipal-services/ws-calculator/src/main/resources/application.properties (1 hunks)
- utilities/Penalty/pom.xml (1 hunks)
- utilities/Penalty/src/main/java/org/egov/PenaltySchedularApplication.java (1 hunks)
- utilities/Penalty/src/main/java/org/egov/config/PenaltyShedularConfiguration.java (1 hunks)
- utilities/Penalty/src/main/java/org/egov/job/PenaltySchedularJob.java (1 hunks)
- utilities/Penalty/src/main/java/org/egov/model/AddPenaltyCriteria.java (1 hunks)
- utilities/Penalty/src/main/java/org/egov/model/PenaltyRequest.java (1 hunks)
- utilities/Penalty/src/main/java/org/egov/model/Tenant.java (1 hunks)
- utilities/Penalty/src/main/java/org/egov/util/MDMSClient.java (1 hunks)
- utilities/Penalty/src/main/java/org/egov/util/PenaltySchedularConstant.java (1 hunks)
- utilities/Penalty/src/main/resources/application.properties (1 hunks)
Files skipped from review due to trivial changes (4)
- business-services/billing-service/src/main/java/org/egov/demand/util/Constants.java
- utilities/Penalty/src/main/java/org/egov/model/Tenant.java
- utilities/Penalty/src/main/java/org/egov/util/PenaltySchedularConstant.java
- utilities/Penalty/src/main/resources/application.properties
Additional comments not posted (21)
municipal-services/ws-calculator/src/main/java/org/egov/wscalculation/web/models/AddPenaltyCriteria.java (1)
15-19: The structure and annotations of this class are appropriate for its intended use case.utilities/Penalty/src/main/java/org/egov/PenaltySchedularApplication.java (1)
7-11: The application setup, including the exclusion ofDataSourceAutoConfiguration, is correctly configured for a scheduler application that likely relies on external services for data interaction.utilities/Penalty/src/main/java/org/egov/model/PenaltyRequest.java (1)
14-20: The structure and annotations ofPenaltyRequestare well-defined, ensuring proper data encapsulation and validation for penalty processing.municipal-services/ws-calculator/src/main/java/org/egov/wscalculation/web/models/PenaltyRequest.java (1)
14-20: The structure and annotations of thisPenaltyRequestclass are consistent with its counterpart in another package, ensuring uniformity and proper data encapsulation across the application.business-services/billing-service/src/main/java/org/egov/demand/model/AggregatedDemandDetailResponse.java (1)
16-40: The structure and annotations ofAggregatedDemandDetailResponseare well-defined, with appropriate use ofBigDecimalfor financial calculations, ensuring accuracy and precision in billing and penalty calculations.utilities/Penalty/src/main/java/org/egov/config/PenaltyShedularConfiguration.java (3)
22-25: Initialization method correctly sets the default timezone.The
initializemethod effectively sets the default timezone using theapp.timezoneproperty, which is a good practice for consistency across various environments.
46-46: Validate the list of division codes for penalty application.Ensure that the division codes listed in
egov.penalty.enabled.division.code.listare valid and exist in the system's configuration or database.
19-19: Ensure proper validation and fallback for configuration properties.It's crucial to validate configuration properties like
app.timezoneandegov.penalty.enabledto ensure they are not misconfigured. Consider adding a fallback mechanism or default values.Also applies to: 43-43
utilities/Penalty/src/main/java/org/egov/util/MDMSClient.java (1)
44-93: Improve error handling and logging in MDMS client.
[REFACTOR_Suggestion]
The methodloadTenantsFromMdmscould improve its error handling and logging practices. Specifically, it should handle potential null values and exceptions more gracefully.- log.info("Error occurred while getting the account to gp mapping from MDMS", e); + log.error("Error occurred while getting the account to gp mapping from MDMS", e);utilities/Penalty/pom.xml (1)
27-27: Verify that thepostgresql.versionproperty is defined in your properties section or parent POM to avoid build issues.Verification successful
The
postgresql.versionproperty is defined in theutilities/Penalty/pom.xmlfile, ensuring that there should be no build issues related to this property.
utilities/Penalty/pom.xml:<postgresql.version>42.3.3</postgresql.version>Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check if postgresql.version is defined grep -r "postgresql.version" .Length of output: 195
municipal-services/ws-calculator/src/main/resources/application.properties (1)
154-157: Ensure that the penalty feature toggle (is.penalty.feature.enable) is well-documented and tested across different environments to avoid unintended feature activations.municipal-services/ws-calculator/src/main/java/org/egov/wscalculation/config/WSCalculationConfiguration.java (1)
259-269: The penalty configuration properties are well-defined. Ensure that the default values and constraints (if any) are documented in the application's configuration guide.business-services/billing-service/src/main/java/org/egov/demand/web/controller/DemandController.java (1)
166-174: The new endpoint for aggregated demand details is correctly implemented. Ensure that the corresponding service methodgetAllDemandshandles exceptions and edge cases effectively.municipal-services/ws-calculator/src/main/java/org/egov/wscalculation/constants/WSCalculationConstant.java (1)
325-328: The new JSON path constants for the water services calculator and penalty are correctly defined. Ensure that these paths are consistent with the MDMS configuration.municipal-services/ws-calculator/src/main/java/org/egov/wscalculation/util/CalculatorUtil.java (1)
397-416: The implementation ofgetPenaltyMasterForTenantIdmethod looks good with proper error handling and logging.business-services/billing-service/src/main/java/org/egov/demand/service/DemandService.java (5)
45-45: Added import forjava.math.RoundingMode.This import is necessary for handling rounding operations in the new method
getAllDemands.
46-46: Updated imports to include more general utilities.This change is likely to support the new functionalities added in the
DemandServiceclass.
57-57: Added imports for various models fromorg.egov.demand.model.These imports are necessary for the new methods and modifications in the existing methods to handle different demand models.
84-88: Added imports for MDMS (Master Data Management System) models.These imports are essential for interacting with the MDMS service, which seems to be a new integration in this service class.
547-755: Added a new methodgetAllDemandsto fetch and process demand data.This method is well-structured and follows good coding practices. However, consider adding more detailed logging for debugging and maintenance purposes.
[REFACTOR_SUGGESTion]+ log.debug("Starting to fetch all demands with criteria: {}", demandCriteria); + log.debug("Completed fetching all demands. Total demands fetched: {}", demands.size());municipal-services/ws-calculator/src/main/java/org/egov/wscalculation/service/DemandService.java (1)
68-69: Ensure that the new importsorg.springframework.http.ResponseEntityandorg.apache.http.HttpStatusare utilized in the code.Verification successful
The new imports
org.springframework.http.ResponseEntityandorg.apache.http.HttpStatusare indeed utilized in the code.
ResponseEntityis used in the methodaddPenalty.HttpStatusis used within theResponseEntityinstantiation.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify if the new imports are used in the code. rg --type java 'ResponseEntity|HttpStatus' municipal-services/ws-calculator/src/main/java/org/egov/wscalculation/service/DemandService.javaLength of output: 509
| "${egov.update.demand.add.penalty}" }, containerFactory = "kafkaListenerContainerFactory") | ||
| public void updateAddPenalty(HashMap<Object, Object> messageData) { | ||
| DemandRequest demandRequest = mapper.convertValue(messageData, DemandRequest.class); | ||
| demandService.updateDemandAddPenalty(demandRequest.getRequestInfo(), demandRequest.getDemands()); | ||
| } |
There was a problem hiding this comment.
Add error handling for message processing.
Consider wrapping the message processing logic in a try-catch block to handle potential exceptions gracefully. This will improve the robustness of the service, especially in cases where the message format might not be as expected or if the updateDemandAddPenalty method encounters an error.
+ try {
DemandRequest demandRequest = mapper.convertValue(messageData, DemandRequest.class);
demandService.updateDemandAddPenalty(demandRequest.getRequestInfo(), demandRequest.getDemands());
+ } catch (Exception e) {
+ log.error("Error processing message for adding penalty: ", e);
+ }Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| "${egov.update.demand.add.penalty}" }, containerFactory = "kafkaListenerContainerFactory") | |
| public void updateAddPenalty(HashMap<Object, Object> messageData) { | |
| DemandRequest demandRequest = mapper.convertValue(messageData, DemandRequest.class); | |
| demandService.updateDemandAddPenalty(demandRequest.getRequestInfo(), demandRequest.getDemands()); | |
| } | |
| "${egov.update.demand.add.penalty}" }, containerFactory = "kafkaListenerContainerFactory") | |
| public void updateAddPenalty(HashMap<Object, Object> messageData) { | |
| try { | |
| DemandRequest demandRequest = mapper.convertValue(messageData, DemandRequest.class); | |
| demandService.updateDemandAddPenalty(demandRequest.getRequestInfo(), demandRequest.getDemands()); | |
| } catch (Exception e) { | |
| log.error("Error processing message for adding penalty: ", e); | |
| } | |
| } |
| MdmsCriteriaReq mdmsCriteriaReq = MdmsCriteriaReq.builder().mdmsCriteria(mdmsCriteria).requestInfo(requestInfo).build(); | ||
| Object res = serviceRequestRepository.fetchResult(getMdmsSearchUrl(), mdmsCriteriaReq); | ||
| if (res == null) { | ||
| throw new CustomException("MDMS_ERROR_FOR_BILLING_FREQUENCY", "ERROR IN FETCHING THE ALLOWED PAYMENT FOR TENANTID " + tenantId); |
There was a problem hiding this comment.
Refine the exception message to be specific to penalty data fetching.
- throw new CustomException("MDMS_ERROR_FOR_BILLING_FREQUENCY", "ERROR IN FETCHING THE ALLOWED PAYMENT FOR TENANTID " + tenantId);
+ throw new CustomException("MDMS_ERROR_FOR_PENALTY_FETCH", "Error fetching penalty data for tenant ID: " + tenantId);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.
| throw new CustomException("MDMS_ERROR_FOR_BILLING_FREQUENCY", "ERROR IN FETCHING THE ALLOWED PAYMENT FOR TENANTID " + tenantId); | |
| throw new CustomException("MDMS_ERROR_FOR_PENALTY_FETCH", "Error fetching penalty data for tenant ID: " + tenantId); |
| if (res == null) { | ||
| throw new CustomException("MDMS_ERROR_FOR_BILLING_FREQUENCY", "ERROR IN FETCHING THE ALLOWED PAYMENT FOR TENANTID " + tenantId); | ||
| } | ||
| log.info("Response", res); |
There was a problem hiding this comment.
Enhance the logging statement for clarity and detail.
- log.info("Response", res);
+ log.info("Received MDMS response for penalty data: {}", res);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.
| log.info("Response", res); | |
| log.info("Received MDMS response for penalty data: {}", res); |
| @@ -30,7 +30,7 @@ | |||
| import java.util.stream.Collectors; | |||
|
|
|||
| import javax.validation.Valid; | |||
|
|
|||
| import io.swagger.models.auth.In; | |||
There was a problem hiding this comment.
Tip
Codebase Verification
The import io.swagger.models.auth.In is not used in the DemandService.java file. Unused imports should be removed to keep the codebase clean.
- Location:
municipal-services/ws-calculator/src/main/java/org/egov/wscalculation/service/DemandService.javaat line 33.
Analysis chain
Ensure that the import io.swagger.models.auth.In is used in the code. Unused imports can clutter the codebase.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify if the import `io.swagger.models.auth.In` is used in the code.
rg --type java 'In' municipal-services/ws-calculator/src/main/java/org/egov/wscalculation/service/DemandService.java
Length of output: 13799
| public List<String> getDemandToAddPenalty(String tenantid,BigInteger penaltyThresholdDate,Integer penaltyApplicableAfterDays){ | ||
| return demandRepository.getDemandsToAddPenalty(tenantid,penaltyThresholdDate,penaltyApplicableAfterDays); | ||
| } | ||
|
|
||
| public ResponseEntity<HttpStatus> addPenalty(@Valid RequestInfo requestInfo, AddPenaltyCriteria addPenaltyCriteria) { | ||
| if(config.isPenaltyEnabled()) { | ||
| if (requestInfo.getUserInfo().equals(null)) { | ||
|
|
||
| } | ||
| List<MasterDetail> masterDetails = new ArrayList<>(); | ||
| MasterDetail masterDetail = new MasterDetail("Penalty", "[?(@)]"); | ||
| masterDetails.add(masterDetail); | ||
| ModuleDetail moduleDetail = ModuleDetail.builder().moduleName("ws-services-calculation").masterDetails(masterDetails).build(); | ||
| List<ModuleDetail> moduleDetails = new ArrayList<>(); | ||
| moduleDetails.add(moduleDetail); | ||
| MdmsCriteria mdmsCriteria = MdmsCriteria.builder().tenantId(addPenaltyCriteria.getTenantId()) | ||
| .moduleDetails(moduleDetails) | ||
| .build(); | ||
| Map<String, Object> paymentMasterData = calculatorUtils.getPenaltyMasterForTenantId(addPenaltyCriteria.getTenantId(), mdmsCriteria, requestInfo); | ||
|
|
||
| Integer rate= (Integer) paymentMasterData.get("rate"); | ||
| String penaltyType = String.valueOf(paymentMasterData.get("type")); | ||
| String penaltySubType = (String) paymentMasterData.get("subType"); | ||
| String startingDay = (String) paymentMasterData.get("startingDay"); | ||
| Integer applicableAfterDays = (Integer) paymentMasterData.get("applicableAfterDays"); | ||
| List<String> demandIds = getDemandToAddPenalty(addPenaltyCriteria.getTenantId(), new BigInteger(config.getPenaltyStartThresholdTime()),applicableAfterDays); | ||
| if (rate > 0) { | ||
| demandIds.stream().forEach(demandId -> { | ||
| Set<String> demandids = new HashSet<>(); | ||
| demandids.add(demandId); | ||
| List<Demand> demands = searchDemandBydemandId(addPenaltyCriteria.getTenantId(), demandids, requestInfo); | ||
| if (!CollectionUtils.isEmpty(demands)) { | ||
| Demand demand = demands.get(0); | ||
| Boolean isPenaltyExistForDemand = demand.getDemandDetails().stream().anyMatch(demandDetail -> { | ||
| return demandDetail.getTaxHeadMasterCode().equalsIgnoreCase(WSCalculationConstant.WS_TIME_PENALTY); | ||
| }); | ||
| if (!isPenaltyExistForDemand) { | ||
| if (!CollectionUtils.isEmpty(demand.getDemandDetails()) && demand.getDemandDetails().size() == 1) { | ||
| demand.setDemandDetails(addTimePenalty(rate, penaltyType, penaltySubType, demand)); | ||
| demands.clear(); | ||
| demands.add(demand); | ||
| DemandRequest demandRequest = DemandRequest.builder().requestInfo(requestInfo).demands(demands).build(); | ||
| producer.push(config.getUpdateAddPenaltytopic(), demandRequest); | ||
| } | ||
| } | ||
| } | ||
| }); | ||
| } else { | ||
| return new ResponseEntity<>(org.springframework.http.HttpStatus.METHOD_NOT_ALLOWED); | ||
| } | ||
| } | ||
| return new ResponseEntity<>(org.springframework.http.HttpStatus.ACCEPTED); | ||
| } |
There was a problem hiding this comment.
Tip
Codebase Verification
The search did not return any test files covering the addPenalty method. This indicates that there might be no unit tests for this method, which is a concern given the importance of ensuring the accuracy of penalty calculations and handling edge cases.
- Please add unit tests for the
addPenaltymethod to verify the penalty calculation logic and ensure it handles all edge cases, such as non-positive rates or missing user info.
Analysis chain
The method addPenalty has been added to handle penalty calculations and updates. It is crucial to ensure that the penalty calculation logic is accurate and that the method handles all edge cases, such as non-positive rates or missing user info.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the penalty calculation logic and edge case handling.
# Test: Search for unit tests covering this method.
rg --type java 'testAddPenalty' municipal-services/ws-calculator/src/test/java/org/egov/wscalculation/service/DemandServiceTest.java
Length of output: 273
Script:
#!/bin/bash
# Description: Verify the penalty calculation logic and edge case handling.
# Test: Search for any test files covering the addPenalty method.
rg --type java 'addPenalty' municipal-services/ws-calculator/src/test/
Length of output: 153
Script:
#!/bin/bash
# Description: Verify the penalty calculation logic and edge case handling.
# Test: Search for any test files covering the addPenalty method in the entire repository.
rg --type java 'addPenalty'
Length of output: 2915
| public String getPenaltyQuery(String tenantId, BigInteger penaltyThresholdDate, Integer daysToBeSubstracted ) { | ||
| //TODO: find out days | ||
| long currentTimeMillis = System.currentTimeMillis(); | ||
| long tenDaysAgoMillis = Instant.ofEpochMilli(currentTimeMillis) | ||
| .minus(daysToBeSubstracted, ChronoUnit.DAYS) | ||
| .toEpochMilli(); | ||
| String subStringQuery ; | ||
| subStringQuery = subQuery + " AND d.tenantid = '"+tenantId+"'" + | ||
| " AND d.createdtime < " + tenDaysAgoMillis + " AND d.taxperiodfrom > " + penaltyThresholdDate; | ||
| String firstStringWhereClause; | ||
| firstStringWhereClause = firstWhereClause + subStringQuery + secondWhereClause + tenantId+"'"+ groupByClause; | ||
| String query ; | ||
| query = selectClause + firstStringWhereClause; | ||
| StringBuilder builder = new StringBuilder(query); | ||
| log.info("Query formed :" + builder.toString()); | ||
| return builder.toString(); |
There was a problem hiding this comment.
Optimize the SQL query generation logic.
The method getPenaltyQuery constructs a SQL query string using string concatenation, which can be error-prone and hard to maintain. Consider using a more robust approach like using a SQL builder library or parameterized queries.
- String subStringQuery = subQuery + " AND d.tenantid = '"+tenantId+"'" +
- " AND d.createdtime < " + tenDaysAgoMillis + " AND d.taxperiodfrom > " + penaltyThresholdDate;
+ String subStringQuery = String.format("%s AND d.tenantid = ? AND d.createdtime < ? AND d.taxperiodfrom > ?", subQuery);Committable suggestion was skipped due low confidence.
| public List<String> getDemandsToAddPenalty(String tenantId, BigInteger penaltyThresholdTime, Integer penaltyApplicableAfterDays) { | ||
| List<Object> preparedStmtList = new ArrayList<>(); | ||
| String query = queryBuilder.getPenaltyQuery(tenantId, penaltyThresholdTime, penaltyApplicableAfterDays); | ||
| log.info("query:"+ query); | ||
| return jdbcTemplate.queryForList(query, String.class); | ||
| } |
There was a problem hiding this comment.
Ensure proper handling of SQL queries and potential SQL injection.
The method getDemandsToAddPenalty directly uses string concatenation to form SQL queries, which could lead to SQL injection vulnerabilities. Use prepared statements or other safer methods to construct queries.
- String query = queryBuilder.getPenaltyQuery(tenantId, penaltyThresholdTime, penaltyApplicableAfterDays);
+ String query = queryBuilder.getPenaltyQuery(tenantId, penaltyThresholdTime, penaltyApplicableAfterDays); // Ensure this method uses prepared statementsCommittable suggestion was skipped due low confidence.
| public BigDecimal roundOffTax (BigDecimal tax) { | ||
|
|
||
| // Round the value up to the next highest integer | ||
| return tax.setScale(0, RoundingMode.HALF_UP); | ||
| } |
There was a problem hiding this comment.
Tip
Codebase Verification
The roundOffTax method in the DemandService class does not appear to have any associated unit tests. This lack of testing coverage means that the method's behavior, especially its compliance with financial rules, is not verified.
- Ensure that the
roundOffTaxmethod is covered by unit tests to validate its functionality and compliance with financial rules.
Analysis chain
The method roundOffTax has been added to round tax values. Ensure that this rounding is consistent with the financial rules applicable to the service.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the rounding logic in `roundOffTax`.
# Test: Search for usage and unit tests of this method.
rg --type java 'roundOffTax' municipal-services/ws-calculator/src/main/java/org/egov/wscalculation/service/DemandService.java
Length of output: 204
Script:
#!/bin/bash
# Description: Verify the usage and unit tests of the `roundOffTax` method.
# Search for all invocations of the `roundOffTax` method in the codebase.
rg --type java 'roundOffTax'
# Search for unit tests that cover the `roundOffTax` method.
rg --type java 'roundOffTax' --glob '*Test.java'
Length of output: 350
Script:
#!/bin/bash
# Description: Verify the presence of unit tests for the `roundOffTax` method.
# List all test files in the relevant directories.
fd --type f --glob '*Test.java' municipal-services/ws-calculator/src/test/java/
# Search for the `roundOffTax` method in the listed test files.
fd --type f --glob '*Test.java' municipal-services/ws-calculator/src/test/java/ --exec rg 'roundOffTax' {}
Length of output: 460
Script:
#!/bin/bash
# Description: Verify the presence of unit tests for the `roundOffTax` method.
# Locate the test directory.
fd --type d 'test'
# Search for the `roundOffTax` method in the test files.
fd --type f --glob '*Test.java' $(fd --type d 'test') --exec rg 'roundOffTax' {}
Length of output: 1504
Script:
#!/bin/bash
# Description: Verify the presence of unit tests for the `roundOffTax` method.
# List all test files in the relevant directories.
fd --type f --glob '*Test.java' business-services/billing-service/src/test/ business-services/egov-apportion-service/src/test/ business-services/egov-hrms/src/test/ core-services/egov-pg-service/src/test/ core-services/egov-user/src/test/ core-services/user-otp/src/test/ frontend/mgramseva/integration_test/ frontend/mgramseva/lib/utils/testing_keys/ frontend/mgramseva/test/ frontend/mgramseva/test_driver/ municipal-services/egov-user-event/src/test/ municipal-services/mgramseva-ifix-adapter/src/test/ municipal-services/property-services/src/test/
# Search for the `roundOffTax` method in the listed test files.
fd --type f --glob '*Test.java' business-services/billing-service/src/test/ business-services/egov-apportion-service/src/test/ business-services/egov-hrms/src/test/ core-services/egov-pg-service/src/test/ core-services/egov-user/src/test/ core-services/user-otp/src/test/ frontend/mgramseva/integration_test/ frontend/mgramseva/lib/utils/testing_keys/ frontend/mgramseva/test/ frontend/mgramseva/test_driver/ municipal-services/egov-user-event/src/test/ municipal-services/mgramseva-ifix-adapter/src/test/ municipal-services/property-services/src/test/ --exec rg 'roundOffTax' {}
Length of output: 5941
| public void addPenaltyEventToWaterCalculator(AddPenaltyCriteria penaltyCriteria) { | ||
| Role role= Role.builder().code(config.getRole()).tenantId(config.getTenantId()).name(config.getRole()).build(); | ||
| List<Role> roles = new ArrayList<>(); | ||
| roles.add(role); | ||
| log.info("role:"+role); | ||
| log.info("roles:"+roles); | ||
| User user = User.builder().userName(config.getUserName()). | ||
| mobileNumber(config.getUserName()). | ||
| uuid(config.getUuid()). | ||
| roles(roles). | ||
| tenantId(config.getTenantId()).build(); | ||
| log.info("userInfo:"+user); | ||
| log.info("penaltyCriteria:"+penaltyCriteria); | ||
| RequestInfo requestInfo= RequestInfo.builder().userInfo(user).build(); | ||
| PenaltyRequest penaltyRequest = PenaltyRequest.builder().requestInfo(requestInfo).addPenaltyCriteria(penaltyCriteria).build(); | ||
| log.info("Posting request to add Penalty for tenantid:" +penaltyCriteria.getTenantId()); | ||
| log.info("Penalty Request"+ penaltyRequest); | ||
| if (penaltyCriteria.getTenantId() != null) { | ||
| if (penaltyCriteria.getTenantId().equalsIgnoreCase("pb.poohlahjgfid")) { | ||
| try { | ||
| HttpHeaders headers = new HttpHeaders(); | ||
| headers.setContentType(MediaType.APPLICATION_JSON); | ||
|
|
||
| HttpEntity<PenaltyRequest> request = new HttpEntity<>(penaltyRequest, headers); | ||
| restTemplate.exchange(getWaterConnnectionAddPennanltyUrl(), HttpMethod.POST,request,Map.class); | ||
|
|
||
| log.info("Posted request to add Penalty for tenant:" + penaltyCriteria.getTenantId()); | ||
| } catch (RestClientException e) { | ||
| log.info("Error while calling to water calculator service for tenant :" + penaltyCriteria.getTenantId() + " ERROR MESSAGE:" + e.getMessage(), e.getCause()); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Review and optimize the logic for adding penalty events.
The method addPenaltyEventToWaterCalculator contains complex logic that could be simplified or broken down into smaller methods to improve readability and maintainability.
- if (penaltyCriteria.getTenantId() != null) {
+ if (StringUtils.hasText(penaltyCriteria.getTenantId())) {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.
| public void addPenaltyEventToWaterCalculator(AddPenaltyCriteria penaltyCriteria) { | |
| Role role= Role.builder().code(config.getRole()).tenantId(config.getTenantId()).name(config.getRole()).build(); | |
| List<Role> roles = new ArrayList<>(); | |
| roles.add(role); | |
| log.info("role:"+role); | |
| log.info("roles:"+roles); | |
| User user = User.builder().userName(config.getUserName()). | |
| mobileNumber(config.getUserName()). | |
| uuid(config.getUuid()). | |
| roles(roles). | |
| tenantId(config.getTenantId()).build(); | |
| log.info("userInfo:"+user); | |
| log.info("penaltyCriteria:"+penaltyCriteria); | |
| RequestInfo requestInfo= RequestInfo.builder().userInfo(user).build(); | |
| PenaltyRequest penaltyRequest = PenaltyRequest.builder().requestInfo(requestInfo).addPenaltyCriteria(penaltyCriteria).build(); | |
| log.info("Posting request to add Penalty for tenantid:" +penaltyCriteria.getTenantId()); | |
| log.info("Penalty Request"+ penaltyRequest); | |
| if (penaltyCriteria.getTenantId() != null) { | |
| if (penaltyCriteria.getTenantId().equalsIgnoreCase("pb.poohlahjgfid")) { | |
| try { | |
| HttpHeaders headers = new HttpHeaders(); | |
| headers.setContentType(MediaType.APPLICATION_JSON); | |
| HttpEntity<PenaltyRequest> request = new HttpEntity<>(penaltyRequest, headers); | |
| restTemplate.exchange(getWaterConnnectionAddPennanltyUrl(), HttpMethod.POST,request,Map.class); | |
| log.info("Posted request to add Penalty for tenant:" + penaltyCriteria.getTenantId()); | |
| } catch (RestClientException e) { | |
| log.info("Error while calling to water calculator service for tenant :" + penaltyCriteria.getTenantId() + " ERROR MESSAGE:" + e.getMessage(), e.getCause()); | |
| } | |
| } | |
| } | |
| public void addPenaltyEventToWaterCalculator(AddPenaltyCriteria penaltyCriteria) { | |
| Role role= Role.builder().code(config.getRole()).tenantId(config.getTenantId()).name(config.getRole()).build(); | |
| List<Role> roles = new ArrayList<>(); | |
| roles.add(role); | |
| log.info("role:"+role); | |
| log.info("roles:"+roles); | |
| User user = User.builder().userName(config.getUserName()). | |
| mobileNumber(config.getUserName()). | |
| uuid(config.getUuid()). | |
| roles(roles). | |
| tenantId(config.getTenantId()).build(); | |
| log.info("userInfo:"+user); | |
| log.info("penaltyCriteria:"+penaltyCriteria); | |
| RequestInfo requestInfo= RequestInfo.builder().userInfo(user).build(); | |
| PenaltyRequest penaltyRequest = PenaltyRequest.builder().requestInfo(requestInfo).addPenaltyCriteria(penaltyCriteria).build(); | |
| log.info("Posting request to add Penalty for tenantid:" +penaltyCriteria.getTenantId()); | |
| log.info("Penalty Request"+ penaltyRequest); | |
| if (StringUtils.hasText(penaltyCriteria.getTenantId())) { | |
| if (penaltyCriteria.getTenantId().equalsIgnoreCase("pb.poohlahjgfid")) { | |
| try { | |
| HttpHeaders headers = new HttpHeaders(); | |
| headers.setContentType(MediaType.APPLICATION_JSON); | |
| HttpEntity<PenaltyRequest> request = new HttpEntity<>(penaltyRequest, headers); | |
| restTemplate.exchange(getWaterConnnectionAddPennanltyUrl(), HttpMethod.POST,request,Map.class); | |
| log.info("Posted request to add Penalty for tenant:" + penaltyCriteria.getTenantId()); | |
| } catch (RestClientException e) { | |
| log.info("Error while calling to water calculator service for tenant :" + penaltyCriteria.getTenantId() + " ERROR MESSAGE:" + e.getMessage(), e.getCause()); | |
| } | |
| } | |
| } |
| @PostMapping("/_addPenalty") | ||
| public ResponseEntity<org.apache.http.HttpStatus> addPenalty(@RequestBody PenaltyRequest penaltyRequest) { | ||
| return demandService.addPenalty(penaltyRequest.getRequestInfo(),penaltyRequest.getAddPenaltyCriteria()); | ||
|
|
||
| } |
There was a problem hiding this comment.
Ensure correct HTTP status codes are used in response.
The method addPenalty should return an appropriate ResponseEntity type instead of org.apache.http.HttpStatus. This could lead to incorrect HTTP response handling.
- public ResponseEntity<org.apache.http.HttpStatus> addPenalty(@RequestBody PenaltyRequest penaltyRequest) {
+ public ResponseEntity<?> addPenalty(@RequestBody PenaltyRequest penaltyRequest) {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.
| @PostMapping("/_addPenalty") | |
| public ResponseEntity<org.apache.http.HttpStatus> addPenalty(@RequestBody PenaltyRequest penaltyRequest) { | |
| return demandService.addPenalty(penaltyRequest.getRequestInfo(),penaltyRequest.getAddPenaltyCriteria()); | |
| } | |
| @PostMapping("/_addPenalty") | |
| public ResponseEntity<?> addPenalty(@RequestBody PenaltyRequest penaltyRequest) { | |
| return demandService.addPenalty(penaltyRequest.getRequestInfo(),penaltyRequest.getAddPenaltyCriteria()); | |
| } |
Summary by CodeRabbit
New Features
Enhancements
Configuration
application.propertieswith penalty-related configurations and logging settings.Documentation